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

'Promise' is undefined on IE11 #1079

Closed
3cp opened this issue Mar 28, 2019 · 14 comments

Comments

Projects
None yet
4 participants
@3cp
Copy link
Member

commented Mar 28, 2019

I'm submitting a bug report

  • Library Version:
    v1.0.0-beta.14

Please tell us about your environment:

  • Operating System:
    Windows

  • Node Version:
    10.15.2

  • NPM Version:
    6.9.0

  • Browser:
    IE11

  • Language:
    all

  • Loader/bundler:
    all

Current behavior:

The default apps (both webpack and cli-bundler) don't work on IE11, showing error 'Promise' is undefined.

  • What is the expected behavior?
    Expect 'Promise' is poly-filled properly in IE11.

  • What is the motivation / use case for changing the behavior?
    This is a regression introduced in #1047 which I didn't test in IE11 properly.

Root Cause: the first line in src/main.js or src/main.ts: import 'any-polyfill'; is not the first code executed in aurelia app, it is BEHIND aurelia-bootstrapper.

aurelia-bootstrapper uses Promise way before user's main.js could do the polyfill.
bootstrapper uses Promise here:
https://github.com/aurelia/bootstrapper/blob/6b8437a8faf73b56baf6b2fdf953e73ef723760c/src/index.js#L7
before loading main module here:
https://github.com/aurelia/bootstrapper/blob/6b8437a8faf73b56baf6b2fdf953e73ef723760c/src/index.js#L162-L163

This timing issue was not a problem before, because we used to using prepend (for built-in bundle), or ProvidePlugin (for webpack) to load bluebird. They happens before aurelia-bootstrapper kicks in.

I am not sure can we fix this timing issue from bootstrapper. Alter the timing? Try load main module before using Promise? The issue is I think aurelia-loader also uses Promise.

If that's not possible, we will have to revert to using prepend/ProvidePlugin to load core-js (current polyfill). But this is less intuitive to end users.

cc @EisenbergEffect

@EisenbergEffect

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

We can't handle this inside the framework. It needs to ensure promise before anything runs. That's why we used prepend before. I think we should just add it back. Or add a cli install step that asks whether they need to support IE11, and then add it conditionally.

@3cp

This comment has been minimized.

Copy link
Member Author

commented Mar 28, 2019

Ok. I tested webpack, this patch works

new ProviderPlugin({
  'Promise': ['core-js', 'Promise']
});

But we had a new issue with cli-bundler now. Latest core-js v3 does NOT provide an umd build like @babel/polyfill did before, I need to figure out how to prepend it...

The worst solution would be revert back from core-js v3 back to deprecated @babel/polyfill.

@EisenbergEffect

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

Why do we need all of core-js? Can't we just add a promise polyfill like bluebird?

@3cp

This comment has been minimized.

Copy link
Member Author

commented Mar 28, 2019

Probably not, since @babel/polyfill = core-js + regenerator-runtime, we use @babel/polyfill before to bring in promise and regenerator-runtime (for babel only, to support transpiled async/await).

Now they are two lines

import 'core-js/stable';
import 'regenerator-runtime/runtime'; // for esnext only

We only need second line in esnext app.
But I am not exactly sure how much babel relies on the core-js polyfill, but we have aurelia-polyfill that covered decent chunk.

Year before, I remember we only have import '@babel/polyfill'; for webpack skeleton, not cli-bundler skeleton.

The question is which promise polyfill to bring back?
The one in core-js is quite good. But I am not sure can I only use part of it.
es6-promise looks like didn't add finally() for existing Promise in browser. stefanpenner/es6-promise#330
Bluebird is bit big, and slow on IE.

@bigopon

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

Maybe a script tag to polyfill.io?

@EisenbergEffect

This comment has been minimized.

Copy link
Member

commented Mar 29, 2019

At what point did we become dependent on the regenerator runtime? We were explicitly not using async/await in order to avoid that type of dependency in vCurrent.

@3cp

This comment has been minimized.

Copy link
Member Author

commented Mar 29, 2019

No, it was to support user app, requested in #959.

@3cp

This comment has been minimized.

Copy link
Member Author

commented Mar 29, 2019

I can change the line to be like

// To use async/await syntax in your code, uncomment next import.
// ... plus guide on npm install, babelrc config.
// import 'regenerator-runtime/runtime'
// 
@EisenbergEffect

This comment has been minimized.

Copy link
Member

commented Mar 29, 2019

Well, we should probably keep it actually. It's not too bad. I guess I forgot or wasn't thinking when we added that. But, I don't think we should change it now.

I'm open to handling promise with whatever polyfill you think makes sense (and works).

@3cp

This comment has been minimized.

Copy link
Member Author

commented Mar 29, 2019

@bigopon that polyfill.io promise is pretty good. I will make a PR later today.

3cp added a commit to 3cp/cli that referenced this issue Mar 29, 2019

fix(skeleton): fix Promise polyfill on IE
Load Promise polyfill before loading aurelia-bootstrapper. The polyfill can be removed if user don't need to support IE.

closes aurelia#1079

3cp added a commit to 3cp/cli that referenced this issue Mar 29, 2019

fix(skeleton): fix Promise polyfill on IE
Load Promise polyfill before loading aurelia-bootstrapper. The polyfill can be removed if user don't need to support IE.

closes aurelia#1079
@avrahamcool

This comment has been minimized.

Copy link
Contributor

commented Apr 1, 2019

the solution to add a script tag for polyfill.io don't resolve the problem for apps that run in closed environments.
is this polyfill also available as an NPM package?

@3cp

This comment has been minimized.

Copy link
Member Author

commented Apr 1, 2019

es6-promise looks like didn't add finally() for existing Promise in browser. stefanpenner/es6-promise#330

We can use es6-promise in prepend, but it got a small problem on patching Promise.prototype.finally(), the author seems not keen to fix it.

@3cp

This comment has been minimized.

Copy link
Member Author

commented Apr 1, 2019

@avrahamcool I am evaluating https://github.com/taylorhakes/promise-polyfill (seems better than es6-promise)

3cp added a commit to 3cp/cli that referenced this issue Apr 1, 2019

fix(skeleton): fix Promise polyfill on IE
Load promise-polyfill before loading aurelia-bootstrapper. The polyfill can be removed if user don't need to support IE.

closes aurelia#1079

3cp added a commit to 3cp/cli that referenced this issue Apr 2, 2019

fix(skeleton): fix Promise polyfill on IE
Load promise-polyfill before loading aurelia-bootstrapper. The polyfill can be removed if user don't need to support IE.

closes aurelia#1079
@avrahamcool

This comment has been minimized.

Copy link
Contributor

commented Apr 9, 2019

looking forward for this to be released.
It'll simplify some of my apps that require IE.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.