-
Notifications
You must be signed in to change notification settings - Fork 190
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Can't find variable: IntersectionObserver #495
Comments
Are you certain of the browser version? This error is because the |
We see the very same error for some IOS Devices which should according to caniuse ship with the polyfill. I implemented a workaround which loads the polyfill on demand: import { useInView as useInViewLib } from 'react-intersection-observer';
import {useEffect, useRef, useState} from 'react';
import type { InViewHookResponse, IntersectionOptions } from 'react-intersection-observer';
const loadIntersectionObserver = () => import('intersection-observer');
const hasNativeIntersectionObserver = typeof IntersectionObserver !== 'undefined';
let isIntersectionObserverAvailable = hasNativeIntersectionObserver;
/**
* React Hooks make it easy to monitor the inView state of your components.
* Call the useInView hook with the (optional) options you need.
* It will return an array containing a ref, the inView status and the current entry.
* Assign the ref to the DOM element you want to monitor, and the hook will report the status.
*
* With lazy polyfill
*/
export const useInView: typeof useInViewLib = hasNativeIntersectionObserver ? useInViewLib : (args?: IntersectionOptions) => {
const noopRef = useRef(null);
const result = useInViewLib(args);
const [isIntersectionObserverReady, setIsIntersectionObserverReady] = useState<boolean>(isIntersectionObserverAvailable);
useEffect(() => {
if (isIntersectionObserverAvailable) {
return;
}
let mounted = true;
loadIntersectionObserver().then(() => {
isIntersectionObserverAvailable = true;
if (mounted) {
setIsIntersectionObserverReady(true);
}
});
return () => {
mounted = false;
}
}, []);
return isIntersectionObserverReady ? result: {
...result,
ref: noopRef as any
} as InViewHookResponse;
} https://codesandbox.io/s/useinview-lazy-0fgbw?file=/src/useInView.tsx |
We saw this issue and could not figure out why iOS and Mac desktop devices that should support them (Safari 14.1 + 14.2) were throwing a same error. We were seeing quite a few reports in Bugsnag. Turns out that IntersectionObserver can be turned off in Webkit Experimental Features menu, this was useful for recreating the issue. Talking to the user who raised the bug it was off by default on their device (they had never accessed that menu before). We fixed by adding the polyfill |
In general @thebuilder I would love to see this module degrade more gracefully if neither native support nor a polyfill is available. I'm working on a website where we consider dropping the polyfill to ship less code, but if this module throws error the site will not render any content at all in older browsers. It would be nice with a sort of mock fallback (perhaps opt-in) where InView just returns true if there's no IntersectionObserver support available so that graceful degradation becomes simpler. |
It would be simple enough to catch the error, but the proper way of handling it on the client is a lot more difficult since it depends on the use case. Maybe it's just used for lazy loading images, in which case it would make sense to just return I'm open to suggestions, but it shouldn't be a breaking change.
|
Just go with #495 (comment) - it works for all cases and won't download the polyfill if it's not needed |
I agree that just defaulting to true would be bad for some of the use cases, sorry!
|
The lazy polyfill is cool, but I'm still wondering if we'd ship less JS overall with a fix in the component itself. |
Less code yes, but you won't get the functionality without the polyfill. It is a nice way of enriching the |
what do you mean by smaller? the lazy old hook is only 200 byte the polyfill is 3.000 byte |
What I'm aiming for is graceful degradation without the functionality in old browsers. I don't want the polyfill :). Currently this component throws without the polyfill and the page renders nothing at all, so what I'm suggesting is a way to tell this component "if the API is not available don't throw, just say always in view" where we use it for lazy-loading images and "don't throw, just say never in view" if we use it for endless scroll. It's just to be able to opt out of the polyfill without breaking the page, allowing it to degrade gracefully so the content is still available. |
graceful degradation will also require some code.. so in the end you might not even save bytes but have to maintain 2 versions of your app:
|
Of course, that's exactly what graceful degradation is about. Although I don't worry much about maintaining for old browsers - just like the principle. Basically means the app or web site has several functional versions with gradually less functionality. I'm working on a website full of articles and promotional content, so not much interactivity but I do care about it being readable. Your module could also detect the absence of the API it depends on and just do console.error() and leave early or something if that's simpler. I don't care a lot about the exact implementation but I find it unfortunate that the site crashes completely if we drop the polyfill. |
Wondering if it would make sense to build the polyfill loading into the library. It could be done as in #495 (comment) - But thinking it should be possible to move it into https://github.com/thebuilder/react-intersection-observer/blob/master/src/observe.ts#L54 if it creates a temporary placeholder It was removed in #10 along time ago, since it was always importing the polyfill - but making it a dynamic import should work. Haven't tried to make a dynamic import in an NPM package, but suppose it should work just fine with Rollup? |
I agree with that no polyfill loading into the libarary. Thinking a situation that app has some libararies using api-IntersectionObserver, app just import polyfill in one place against in every library for reducing app bundle size. It is the duty of app developer to ensure app availability, not of library maintainer. |
What about moving #495 (comment) into a separate file? That way the user can choose:
I can create a PR if you like |
I was thinking something like this - My main concern is how editors auto import handles it. You'll most likely have to make the choice everytime you import. I'm also inclined to make a version 9.0.0 that does the feature detection and returns |
Returning always true would silently break the UX on a lot of pages without any error logs.. You are right that devs would need to make a decision. But that's also true for other options like rootMargin. The difference here is that the API and effects of the polyfilled hook would be exactly the same like the original hook. That makes changing a previous decision quite easy - just run search/replace to add or remove the polyfill for the codebase |
I've created a PR to solve this with an optional fallback InView value. This doesn't break existing implementations, as it's an opt in feature. It could also make sense to expand the readme to include an example of the @jantimon polyfill solution. |
browser - Mobile Safari 14.1.1
browser.name -- Mobile Safari
device -- iPhone
device.famil - iPhone
os - iOS 14.6
os.name - iOS
Additional context Add any other context about the problem here.

The text was updated successfully, but these errors were encountered: