cornerstone icon indicating copy to clipboard operation
cornerstone copied to clipboard

regeneratorRuntime is not defined

Open sacr3dc0w opened this issue 4 years ago • 10 comments

Just rolled out #1669 for testing and found async/await broke for a custom module used on our stores. If anyone encounters this issue, adding import 'regenerator-runtime/runtime'; at the top of the module resolved that issue for me. Hope this helps anyone else.

sacr3dc0w avatar Jun 17 '20 02:06 sacr3dc0w

In which browser(s) did it break? I wonder if we should add an async/await feature check to the polyfill script.

bookernath avatar Jun 17 '20 02:06 bookernath

  • Chrome 83.0.4103.97
  • Firefox 77.0.1
  • Safari 13.1.1

sacr3dc0w avatar Jun 17 '20 02:06 sacr3dc0w

Module code

export default async function (context) {
    // Import all the navigation events
    const navObj = await import('./navigation');
    const initNav = navObj.default;
    // Initiate nav events
    initNav();

    // Start listrak tracking
    const listrakObj = await import('./listrak');
    const initListrak = listrakObj.default;
    // Initiate listrak events
    initListrak(context, 'Modal-New');
}

sacr3dc0w avatar Jun 17 '20 02:06 sacr3dc0w

I appreciate you raising this and especially for other folks that may run into it. I think your current workaround of including the polyfill directly in your code is probably the best solution for the moment. I'll keep thinking about it though.

bookernath avatar Jun 17 '20 16:06 bookernath

@bookernath

Root Cause This is because of a mismatch between the Stencil code that feature-checks to load the polyfills and the preset-env targets. The preset-env works like this:

  1. Babel figures out what browsers match your target string
  2. It determines which syntax features are not supported by each of those browsers and unites all these sets
  3. It loads the plugins for each of these transformations

So your preset-env target string is causing async/await to get transformed to use of regenerator-runtime. The bundle always contains code that needs regenerator-runtime available in the runtime environment.

But your feature check in Stencil is skipping loading all polyfills, including regenerator-runtime on modern browsers. These browsers support all the features you check for, but they are also receiving the one bundle that was created with regenerator-runtime calls in it.

You would get similar problems for any other syntax transformation that requires runtime support.

Proposed Solution The need for runtime feature checking is pretty small given that Babel is already only emitting polyfill imports needed for the lowest common denominator (see https://babeljs.io/docs/en/babel-preset-env#usebuiltins, you are using the "entry" option).

I suggest you remove it and just let Babel manage what polyfills are loaded. Just import the polyfills.js from app.js

There are open issues on Babel right now still for supporting runtime feature detection with preset-env. Currently these approaches are mutually exclusive. It is a "necessary evil" to load unnecessary polyfills on the "best" browsers in your "targets" when using preset-env. Please see https://github.com/babel/babel/issues/11425#issuecomment-626386870

If you want to do more advanced polyfill loading, you are going to need to come up with a complex workaround:

  • You could try to build one output bundle for each "browser class" you want to work with and dynamically load the correct one. The problem here is of course that there is no easy way to detect which "targets" a browser matches at runtime (that is why feature detection is a thing in the first place)
  • So: The only way to ensure correctness then when using preset-env is for you to modify preset-env to add runtime feature detection around polyfill loading (and I hope contribute this enhancement back to Babel).
  • Otherwise: you need to stop using preset-env and build an ad-hoc polyfill loader. This is a much more complex version of what you already tried to build, which is going to require building your own Babel preset, because you need to load syntax transformer plugins, not just polyfills. You need to have consistent logic for matching them up: if you transform async/await, you need to load regenerator-runtime unconditionally. The "best" solution would involve also using webpack to build bundles for the powerset of syntax transformations that require runtime support and using feature detection on syntax support at runtime to load the correct bundle. This is because syntax transformation is done at compile time, so you need one bundle for each possible combination of runtime-support-needing syntax. Here is how you do syntax detection:
       try { eval('async function a() {}'); console.log('works'); } catch (e) { console.log('does not work'); } // logs "works" in a modern browser
       try { eval('foobar {}'); console.log('works'); } catch (e) { console.log('does not work'); } // logs "does not work" in any browser
    

I don't think you should spend too much time on this. Try measuring the actual performance impact of loading the polyfill code with the current target string and a target string that only matches your browser. I doubt it is enough to warrant all the hassle of actually implementing this correctly. The fact that none of the Babel contributors have prioritized improving preset-env shows how unnecessary this really is.

Final Note Telling your users to "just import it while I figure it out" has an unprofessional tone. Stencil should work out-of-box for syntax that is already standardized like async/await. We shouldn't need a workaround, however well documented, to make it work. We use async/await on every theme and it is silly that we would need to do this as a "set up" step each time, especially considering we didn't have to do this in the past.

joeldentici avatar Aug 04 '20 21:08 joeldentici

In which browser(s) did it break? I wonder if we should add an async/await feature check to the polyfill script.

This wouldn't work. Regenerator runtime is already in the bundled output. You need the runtime check + separate bundles (one with async/await compiled, one without) as I mentioned in my previous comment.

Doing it correctly means also going and finding any other standard syntax transformations (the ones preset-env supports) that rely on runtime support. I'm not sure if there are any, but I suggest looking into that, otherwise you will be playing "catch up".

You could determine a browserlist query that only matches browsers supporting it and compile another bundle.

Also, to make sure it is clear: There is no easy way to prevent compilation of async/await in preset-env. You would need a target string that only includes browsers that support it.

If you want an approach that might work most of the time, you can use:

https://github.com/browserslist/browserslist-useragent-regexp

That compiles a browserlist query to a regular expression for user agents. You could use the regular expression on the server to serve the correct bundle in the first place, or on client to determine which to load.

This seems much more feasible than the really fleshed out solutions I discussed before.

joeldentici avatar Aug 04 '20 21:08 joeldentici

Thanks for your feedback @joeldentici. I'll share it with our team (CC @bc-themes ) so we can figure out the best way forward.

I'm supportive of any solution that keeps our bundle size in check for shoppers on modern browsers 👍

bookernath avatar Aug 04 '20 21:08 bookernath

Are there any updates? We are also running into this issue. Adding import 'regenerator-runtime/runtime'; at the top of every custom module is not an ideal solution.

kylebolstad avatar Apr 07 '21 01:04 kylebolstad

Are there any updates? We are also running into this issue. Adding import 'regenerator-runtime/runtime'; at the top of every custom module is not an ideal solution.

rajguru827 avatar Nov 30 '21 08:11 rajguru827

why has this issue been closed? still, I am facing the same issue with the latest cornerstone theme.

rajguru827 avatar Jul 28 '22 10:07 rajguru827