Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

@neutrinojs/library compiles down to IE9 but doesn't include polyfills #1279

Closed
edmorley opened this issue Jan 12, 2019 · 5 comments
Closed
Labels
Milestone

Comments

@edmorley
Copy link
Member

edmorley commented Jan 12, 2019

(Split out of #1251 (comment))

As of #994, @neutrinojs/library generates output that is compatible with IE9 (the idea being to make sure it's fully ES5 compatible). However since Babel has been told to target IE9, it wants to use regenerator-runtime (to polyfill async functions) which isn't available unless users manually import @babel/polyfill in their entrypoints. This results in:
ReferenceError: regeneratorRuntime is not defined

...even though for many people regenerator-runtime is actually unnecessary given the widespread async support in modern browsers (https://caniuse.com/#feat=async-functions).

The options appear to be:
a) Document the need to add a @babel/polyfill import to the entrypoint when using @neutrinojs/library
b) Automatically add a @babel/polyfill import to entrypoints when using @neutrinojs/library
c) Partially revert #994 and default to last 2 browser versions like before, but add an option (or document) how to generate IE9 compatible output for those that need it.
d) Leave the default at 'ie 9' (and do either A or B for people who use that default), but also make it easier to override the browsers list when using @neutrinojs/library, since it doesn't have a shorthand helper like @neutrinojs/web does. This would avoid having to use boilerplate like:

    ['@neutrinojs/library', {
      // ...
      babel: {
        presets: [
          [
            require.resolve('@babel/preset-env'),
            {
              debug: false,
              useBuiltIns: 'entry',
              targets: {
                browsers: [
                  'last 2 Chrome versions',
                  'last 2 Firefox versions',
                  'last 2 Edge versions',
                  'last 2 Opera versions',
                  'last 2 Safari versions',
                  'last 2 iOS versions'
                ],
              }
            }
          ]
        ],
      },
    }],

@neutrinojs/core-contributors - thoughts?

My inclination is that @neutrinojs/library should not support Internet Explorer by default. ie: We should do option (c) above - or at least raise the target browser version to something inbetween IE9 and what it was before ("last 2 versions").

@edmorley edmorley added the bug label Jan 12, 2019
@edmorley edmorley added this to the v9 milestone Jan 12, 2019
@edmorley edmorley changed the title @neutrinojs/library compile down to IE9 but doesn't include polyfills ("ReferenceError: regeneratorRuntime is not defined") @neutrinojs/library compiles down to IE9 but doesn't include polyfills Jan 12, 2019
@edmorley
Copy link
Member Author

edmorley commented Feb 2, 2019

Does anyone have a preference as to which of the options we pick above?

@timkelty
Copy link
Contributor

timkelty commented Feb 4, 2019

My inclination is that @neutrinojs/library should not support Internet Explorer by default. ie: We should do option (c) above - or at least raise the target browser version to something inbetween IE9 and what it was before ("last 2 versions").

I agree with defaulting to "last 2 versions" and have a note in the docs for adding IE9 support.


Also, a note regarding (d):

One possible alternative to the targets boilerplate is to not set it explicitly at all in library and web options, and instead generate a .browserslistrc file.

Personally, I do this in my @neutrinojs/web projects (pass targets: false and use a file: https://github.com/neutrinojs/neutrino/blob/master/packages/web/index.js#L114-L125) for easier interoperability.

It departs a bit from .neutrinorc.js being the "single source of truth", so I'm not sure if it's a good solution for everyone, but thought I'd mention it.

Either way, I think it would still be good to expose targets as an option for @neutrinojs/library, for greater parity with @neutrinojs/web.

@eliperelman
Copy link
Member

One thing to keep in mind is that this preset can build libraries for both Node.js and the browser. Should the output be ES5 for both targets? Use Node.js 8.10 as the Node.js base makes it inconsistent for the output compared to the browser build.

Since these are libraries, opting for broad support by default is the greater need (I think). Making both ES5 sounds correct, but also adding the same override mechanism as the other presets would be good to have.

Regarding the polyfill, we do not want to install it ourselves since it needs to be imported by the application, and we have had troubles in the past with dependencies like that. I think documentation may be the best path here. More thoughts?

@edmorley
Copy link
Member Author

I agree with perhaps defaulting to broad support, however I think "IE9" is a little too broad?

@eliperelman
Copy link
Member

From Babel:

Sidenote, if no targets are specified, @babel/preset-env will transform all ECMAScript 2015+ code by default.

This, combined with forceAllTransforms, may be a good choice to compile to ES5 consistently.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

No branches or pull requests

3 participants