Skip to content
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

Closed
gauravverma029 opened this issue Aug 3, 2021 · 19 comments · Fixed by #521
Closed

Can't find variable: IntersectionObserver #495

gauravverma029 opened this issue Aug 3, 2021 · 19 comments · Fixed by #521

Comments

@gauravverma029
Copy link

browser - Mobile Safari 14.1.1
browser.name -- Mobile Safari
device -- iPhone
device.famil - iPhone
os - iOS 14.6
os.name - iOS

  • Browser [e.g. stock browser, safari]
  • Version [e.g. 22]

Additional context Add any other context about the problem here.
Screenshot 2021-08-03 at 10 13 21

@thebuilder
Copy link
Owner

Are you certain of the browser version? This error is because the IntersectionObserver doesn't exist on the window object, and would normally indicate a missing polyfill.

@jantimon
Copy link

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

@jonjhiggins
Copy link

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).

Uploading Screenshot 2021-08-31 at 12.43.20.png…

We fixed by adding the polyfill

@hallvors
Copy link

hallvors commented Oct 1, 2021

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.

@thebuilder
Copy link
Owner

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 true. But if it's as a sentinel in an infinite scrolling list you'd have a big problem.

I'm open to suggestions, but it shouldn't be a breaking change.

  • Global fallback value if it fails?
  • Option per instance?
  • How do you know it failed - You'd need an error handler, otherwise we can't keep track of how widespread the issue is.

@jantimon
Copy link

jantimon commented Oct 1, 2021

Just go with #495 (comment) - it works for all cases and won't download the polyfill if it's not needed

@hallvors
Copy link

hallvors commented Oct 1, 2021

I agree that just defaulting to true would be bad for some of the use cases, sorry!

<InView noSupportValue={true} > … perhaps?

@hallvors
Copy link

hallvors commented Oct 1, 2021

The lazy polyfill is cool, but I'm still wondering if we'd ship less JS overall with a fix in the component itself.

@thebuilder
Copy link
Owner

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 useInView. Depending on the setup you are using, you could also import('intersection-observer') before rendering React (basic example in readme).

@jantimon
Copy link

jantimon commented Oct 2, 2021

what do you mean by smaller?

the lazy old hook is only 200 byte the polyfill is 3.000 byte

@hallvors
Copy link

hallvors commented Oct 2, 2021

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.

@jantimon
Copy link

jantimon commented Oct 2, 2021

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:

  • app with all features
  • app with no lazy loading, no infinite scroll, no special sticky elements

@hallvors
Copy link

hallvors commented Oct 2, 2021

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.

@thebuilder
Copy link
Owner

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 observer while fetching the polyfill. Would add some overhead, but might be worth it to ensure it works in Safari.

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?

@AdvancedCat
Copy link

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.

@jantimon
Copy link

jantimon commented Nov 1, 2021

What about moving #495 (comment) into a separate file?

That way the user can choose:

  1. import { useInView } from 'react-intersection-observer' -> no polyfill (and no overhead for polyfill loading / placeholder)
  2. import { useInView } from 'react-intersection-observer/polyfilled' -> polyfill on demand (with minimal overhead for lazy loading the polyfill)

I can create a PR if you like

@thebuilder
Copy link
Owner

What about moving #495 (comment) into a separate file?

That way the user can choose:

  1. import { useInView } from 'react-intersection-observer' -> no polyfill (and no overhead for polyfill loading / placeholder)
  2. import { useInView } from 'react-intersection-observer/polyfilled' -> polyfill on demand (with minimal overhead for lazy loading the polyfill)

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 true if unsupported.

@jantimon
Copy link

jantimon commented Nov 1, 2021

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.
It would even use the code of the original hook.

That makes changing a previous decision quite easy - just run search/replace to add or remove the polyfill for the codebase

@thebuilder
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants