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

approach for adding polyfills #2177

Closed
jole78 opened this issue Sep 20, 2017 · 23 comments
Closed

approach for adding polyfills #2177

jole78 opened this issue Sep 20, 2017 · 23 comments

Comments

@jole78
Copy link

jole78 commented Sep 20, 2017

UPDATE: below is the original question that I had...which turned out to be a that I required some polyfills so the discussion around that is at the bottom.

// original question //
Hey,

I can´t seem to get my head around a problem I'm facing with IE 11 (I think). It works in all other browsers I've tested (might be an issue on safari on macOS also...don't have a mac to test it on but indications lead in that direction).

I've deployed a site to netlify (also tested in on surge) but when I test it with IE 11 I get a bunch of console errors pointing at 'webpackJsonp' is undefined.

The site can be seen at http://efficient-hospital.surge.sh/ and https://59c226530752d06f47810f46--abyra.netlify.com/ which both run the same code and gatsby version 1.9.24

Since this error causes the JS to "halt" the site isn't working at all in IE 11.
Tried to test it out locally also but IE 11 doesn't work at all it seems. I can't understand why though. I only get this error (in IE 11 I should say...it works in chrome or Edge or...). Unable to get property 'listeners' of undefined or null reference

Oh yeah, the production site http://www.abyra.se also has issues with IE 11 but they seem different (this runs a previous version of gatsby namely 1.8.4).
Here it "says" Invalid character and points to this...
function patch() {
var head = document.querySelector(head);

Any ideas or hints?

@jbolda
Copy link
Contributor

jbolda commented Sep 20, 2017

This sounds related to #1991. I would recommend upgrading to get that fix.

@jole78
Copy link
Author

jole78 commented Sep 20, 2017

@jbolda yeah...saw that one. However that PR seems to be in master already so it should be on 1.9.24 which I'm using...correct??

@jbolda
Copy link
Contributor

jbolda commented Sep 20, 2017

If you look at the commit, the tag shows 1.9.39: 0d8fc71

@jole78
Copy link
Author

jole78 commented Sep 20, 2017

just looked in my yarn.lock file which says

gatsby@^1.9.24:
  version "1.9.39"
  resolved "https://registry.yarnpkg.com/gatsby/-/gatsby-1.9.39.tgz#23afd3083e195d7255d615ebe19b4d784b1770b2"

so that should mean that I'm on 1.9.39
I also forcefully updated my package.json to be on >1.9.39 but the lock file still says pretty much the same, e.g.

gatsby@^1.9.39:
  version "1.9.39"
  resolved "https://registry.yarnpkg.com/gatsby/-/gatsby-1.9.39.tgz#23afd3083e195d7255d615ebe19b4d784b1770b2"

anyway...the error still persist I'm afraid

@jole78
Copy link
Author

jole78 commented Sep 20, 2017

small update.
I just did a small test where I branched off master instead and updated gatsby to 1.9.39 there and that seemed to work for IE 11. (that didn't work either before)
So I looked at my commits since master up to now and the only "strange" things I saw where that I was using some async/await code + fetch. Does gatsby have support for async/await + fetch OOB or do I have to polyfill it for IE (and perhaps others?). Babel didn't complain so...???

@KyleAMathews
Copy link
Contributor

Yeah first thing to check is if this happens on a vanilla Gatsby site.

If you're using async/await then yeah, you'd need to compile that to something older browser can understand.

@jole78
Copy link
Author

jole78 commented Sep 21, 2017

@KyleAMathews I have had a little progress on this. But now I'm facing some other issues.

Object doesn't support property or method 'normalize' (string.normalize I guess). That error comes from my code.

and then we have Object doesn't support property or method 'assign' which is used in a lib I'm using.

also fetch is undefined

all of these only appear in IE11

The question is...which I've had a little trouble finding out, what does gatsby actually support OOB and how can I/should I polyfill what's missing and how should I modify babel if that's needed. I couldn't really find any of these things in the docs and would of course be willing to PR those things if needed but now I'm not sure how.

@KyleAMathews
Copy link
Contributor

A vanilla gatsby site should work out of the box in IE 11. If it doesn't, would love to hear! Any code you write is up to you to make sure it works in older browsers.

@jole78
Copy link
Author

jole78 commented Sep 21, 2017

@KyleAMathews yeah I know it’s my code or my deps that are causing this and in a “vanilla” react webpack app or something I know how to polyfill and what Babel things are needed. But this is gatsby and thus things work a bit different I guess? Or does it?
That was my question really. I would love to help writing docs around this, like “how to add polyfills” or “how to modify babel” but the problem is I don’t know how to do it myself with gatsby. I have read some PR and tried looking at the code but I can’t seem to really grasp it. It looks like there are some Babel Plugins and Presets that come included with gatsby or are they just available?
This is the only thing I found. Specifically things around how to add polyfills and what polyfills are already included (via babel-preset-env) are missing I guess. In a vanilla react app I could just import whatwg-fetch to get fetch polyfilled...does it work the same with gatsby or should one write a plugin for it?? html.js won’t work I guess??

The biggest question probably is...what is a “vanilla” gatsby site really?
CRA has a pretty opinionated view of what a CRA vanilla app supports and thus documented.
I would also love to help to document that for gatsby...think it could help others.
I just don't know where to start...

Keep it up Kyle. You rock!

@jole78
Copy link
Author

jole78 commented Sep 22, 2017

I was thinking around polyfills in general. Wouldn't it be ok if gatsby used the same approach as CRA (sort of at least) with something like a polyfills.js (or something like that). I mean, in CRA that file is not changeable but if gatsby allowed the user to provide such a file (or something along those lines) then that would be added at the correct place in the webpack entry array, CRA does it like this. From what I could see now the only way to control any sort of polyfills is this in gatsby??

@jole78
Copy link
Author

jole78 commented Sep 22, 2017

Here's an example that I got working at least.
In gatsby-node.js

exports.modifyWebpackConfig = ({ config, stage }) => {
    switch (stage) {
        case "build-javascript":
            const app = config._config.entry.app;
            config._config.entry.app = [require.resolve("./polyfills"), app];

            break;
        default:
            break;
    }

    return config;
};

and polyfills.js (in root of app) which is basically a copy/paste from CRA without the Promise stuff:

// fetch() polyfill for making API calls.
require("whatwg-fetch");

// Object.assign() is commonly used with React.
// It will use the native implementation if it's present and isn't buggy.
Object.assign = require("object-assign");

so, this works...but as I was saying...wouldn't it be nice if gatsby did this and if the user supplied a polyfills.js or maybe a gatsby-polyfills.js file the that would be "injected" in the correct places.

This only works for production builds now (but that was my core use case so...)

@jole78 jole78 changed the title 'webpackJsonp' is undefined approach for adding polyfills Sep 22, 2017
@KyleAMathews
Copy link
Contributor

Oh hmmm… yeah I can see the value to adding the same polyfills they do. We add a promise polyfill but there list is better. We could even just include react-scripts and require their polyfills file.

@jole78
Copy link
Author

jole78 commented Sep 22, 2017

Do whatever you seem to fit gatsby...but the issue with CRA is that you can’t alter anything...that’s what I really like about gatsby (among other things of course). So please keep the option to let the user add their own polyfills in an easy way. The way I did it seemed a bit hacky since you had to know the name of the key in entry etc...I could try to make a PR out of my ideas and see where that gets us?

@KyleAMathews
Copy link
Contributor

this is what we do currently for the promise polyfill

Yeah actually the more I think about it… if you want to use fetch/object.assign and support older browsers that should be up to you. We polyfill promises since we use it in Gatsby's runtime.

To add your own polyfills, add a gatsby-browser.js in your site and implement onClientEntry as that's called as soon as Gatsby starts.

@jole78
Copy link
Author

jole78 commented Sep 22, 2017

ok...? And just require a polyfills.js file or what not in that function or?? Didn’t quite get the use case for onClientEntry from the docs. If it works...then by all means it’s as god fit as any I guess. I’ll be happy to supply some documentation around polyfills if that works since I think others might benefit from it. I’ll close this issue now and we’ll see how it all plays out. Thanks for the input Kyle!

@jole78 jole78 closed this as completed Sep 22, 2017
@wiesson
Copy link

wiesson commented Sep 30, 2017

In order to use fetch, I added installed isomorphic-fetch (yarn add isomorphic-fetch)

gatsby-node.js

exports.onPreBootstrap = () => {
  require('isomorphic-fetch');
};

otherwise, the build failed because fetch is not defined. Is that sufficient?

@robwierzbowski
Copy link

robwierzbowski commented Mar 14, 2018

onClientEntry doesn't seem to solve the problem described in facebook/react#8379 (comment). In order for React and React DOM to work together they both need to have the same version of Symbol set; polyfilling in the gatsby-browser file or in the layouts/index file doesn't seem to be early enough in the process.

Re-defining the webpack entry point to contain a polyfill file would probably be the most certain way to handle this, but I haven't been able to find documentation or examples for how to do this in Gatsby yet. Any suggestions?

@robwierzbowski
Copy link

Blargh, it was unrelated code! We're good :).

@robwierzbowski
Copy link

robwierzbowski commented Mar 14, 2018

My issue was I was using code that needed to be polyfilled in gatsby-browser.js. If I set the babel polyfill in gatsby-browser, I triggered the conditions for the IE11 error referenced above. I solved the problem by moving all fetching and promise based code into the layout component.

@milesw
Copy link

milesw commented Apr 18, 2018

Here's how I finally got babel-polyfill working with Gatsby 1.x to fix React issue #8379 in IE11:

gatsby-browser.js

import 'babel-polyfill'

exports.onClientEntry = () => {
  // Don't need to do anything here, but if you don't 
  // export something, the import won't work.
}

alvis added a commit to alvis/gatsby-webapp that referenced this issue Apr 23, 2018
alvis added a commit to alvis/gatsby-webapp that referenced this issue Apr 23, 2018
alvis added a commit to alvis/gatsby-webapp that referenced this issue Apr 26, 2018
alvis added a commit to alvis/gatsby-webapp that referenced this issue Apr 26, 2018
alvis added a commit to alvis/gatsby-webapp that referenced this issue Apr 26, 2018
alvis added a commit to alvis/gatsby-webapp that referenced this issue Apr 27, 2018
alvis added a commit to alvis/gatsby-webapp that referenced this issue Apr 30, 2018
alvis added a commit to alvis/gatsby-webapp that referenced this issue Apr 30, 2018
alvis added a commit to alvis/gatsby-webapp that referenced this issue Apr 30, 2018
alvis added a commit to alvis/gatsby-webapp that referenced this issue Apr 30, 2018
alvis added a commit to alvis/gatsby-webapp that referenced this issue May 3, 2018
alvis added a commit to alvis/gatsby-webapp that referenced this issue May 3, 2018
@szimek
Copy link
Contributor

szimek commented Jun 28, 2018

Maybe someone will find it useful. We use polyfill.io service to conditionally include required polyfills:

exports.onClientEntry = () => {
  return new Promise((resolve, reject) => {
    window.__polyfillio__ = () => {
      resolve();
    };

    const features = [];

    if (!('Intl' in window)) {
      const locale = window.location.pathname.split('/')[1];
      features.push(`Intl.~locale.${locale}`);
    }

    if (!('fetch' in window)) {
      features.push('fetch');
    }

    // ... detect other missing features

    if (features.length) {
      const s = document.createElement('script');
      s.src = `https://cdn.polyfill.io/v2/polyfill.min.js?features=${features.join(
        ',',
      )}&rum=1&flags=always&callback=__polyfillio__`;
      s.async = true;
      s.onerror = reject;
      document.head.appendChild(s);
    } else {
      resolve();
    }
  });
};

@jonkwheeler
Copy link

@szimek ^^ this was helpful to me. One note, I had to add flags=gated,always (note the gated) part to get this to work in IE11. Noticed this from here.

@antoinerousseau
Copy link
Contributor

is importing babel-polyfill still required for older browsers in current Gatsby version?

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

No branches or pull requests

9 participants