Modify IIFE to safely execute in Node environments

[kohlmannj]

Description

This PR modifies the polyfill's IIFE wrapper to safely allow execution within a Node environment without window and document globals. The modified IIFE uses a combination of these techniques:

• IIFE for non-browser global environments
• Pass either window or this to the wrapped function, as seen in jQuery's wrapper

Motivation + Background

I've been working on an internal project which uses this polyfill to support Safari on macOS and iOS for our usages of a React <Observer> component (@kohlmannj/react-intersection-observer).

I started by statically importing the polyfill (e.g. import 'intersection-observer';), but found that our webpack and Node-based pre-render process (without JSDOM, so without window and document globals) exited with an error due to the polyfill's IIFE wrapper.

I then tried a few variations of the theme of dynamically import()ing of the polyfill client-side, and only in browsers that needed it. While this worked, it added significant complexity to our usage of the <Observer> component (since applying the polyfill is a global side effect, I identified some tricky React render order concerns).

Therefore, I finally went back to this polyfill to see if I could address the root cause of the Node pre-render's failure. I then researched and combined the two generally accepted approaches mentioned above to create the modified IIFE.

Finally, I noted that the test suite in intersection-observer-test.html passed all tests when run in Safari 12.0.2 (14606.3.4) on macOS Mojave 10.14.2. This suggests that the modifications have no impact on the polyfill's correctness when applied client-side.

[kohlmannj]

FYI, I have linked my GitHub account with a newly created W3C account! 🎉

[dontcallmedom]

Marked as non substantive for IPR from ash-nazg.

[kohlmannj]

@dontcallmedom Thank you! Excited to see this merged soon as well 😅😄

[winsandymyint]

Could someone please merge this PR to master? We're facing an error due to the polyfill's IIFE wrapper.

[philipwalton]

This is a duplicate of #279. @kohlmannj in your opinion is there any reason to prefer one over the other?

[kohlmannj]

@philipwalton Hi! A few things, I guess:

1. Compared to #279, this PR has a much smaller, more comprehendible diff (this is minor, however)
2. The approach from this PR follows a well-known IIFE convention (as seen in jQuery)
3. The refactor of the IIFE contents from window to root is beneficial since this code no longer assumes the name of a particular global object
4. Related, the change needed to support the globalThis proposal in the future will be one line: https://github.com/w3c/IntersectionObserver/pull/339/files#r278318268

[philipwalton]

From my perspective, the primary purpose of both of these PRs it to allow the code to be imported/required in a non-window environment and not error in the initial evaluation.

If imported in a non-window environment, I don't think it should be adding symbols to the global object if it's not actually going to be used.

My preferred solution would be one that lets it evaluate successfully in any JavaScript environment but only run (i.e. modify global state) in a window environment.

[kohlmannj]

@philipwalton Ah, I see what you mean now — it's "execute safely" (this PR) vs. "don't execute" (#279).

In that case, I'm okay with closing this PR in favor of #279. I would prefer a slightly more comprehensive check in #279, however, such as this check from tuxsudo/is-in-browser:

const isBrowser = typeof window === "object"
    && typeof document === 'object'
    && document.nodeType === 9;

Here my motivation is to make that "are we in a web browser" check just a little bit more specific, so that the polyfill doesn't inadvertently execute in a browser-like environment that isn't quite right. (For example, consider a naïve "browser environment" mock in Node: maybe global window exists, but global document doesn't, or document has the wrong nodeType, etc.)

Thank you for clarifying 👍🏼

[philipwalton]

A fix for this should be in 0.7.0.