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

[differential loading + web workers] es5 worker doesn't load polyfills #15033

Closed
j2L4e opened this issue Jul 10, 2019 · 9 comments
Closed

[differential loading + web workers] es5 worker doesn't load polyfills #15033

j2L4e opened this issue Jul 10, 2019 · 9 comments

Comments

@j2L4e
Copy link

j2L4e commented Jul 10, 2019

🐞 Bug report

Command

ng build --prod

Is this a regression?

No

Description

The es5 version of web workers doesn't load the polyfills the main application bundle loads. I'm experiencing this with Promise in IE11. The non-worker bundle runs fine as Promise is properly polyfilled. The web worker doesn't run, however, because Promise is undefined.

You can easily work around this using import 'core-js/features/promise' but in my opinion main code and worker code should be bundled in a consistent manner.

🔬 Minimal Reproduction

ng new ie11-promise-worker-repro
cd ie11-promise-worker-repro
ng g web-worker myworker
echo "Promise.resolve().then(() => console.log('I totally work!'))" > ./src/app/myworker.worker.ts
echo "const worker = new Worker('./myworker.worker', { type: 'module' });" > ./src/app/app.component.ts

ng b --prod

npx http-server ./dist/ie11-promise-worker-repro

# open with IE11, you should see it crash when the worker is run.

🔥 Exception or Error


Promise is undefined

🌍 Your Environment



Angular CLI: 8.0.4
Node: 12.6.0
OS: linux x64
Angular: 8.0.2
... animations, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.800.4
@angular-devkit/build-angular     0.800.4
@angular-devkit/build-optimizer   0.800.4
@angular-devkit/build-webpack     0.800.4
@angular-devkit/core              8.0.4
@angular-devkit/schematics        8.0.4
@angular/cdk                      8.0.1
@angular/cli                      8.0.4
@angular/flex-layout              8.0.0-beta.26
@angular/material                 8.0.1
@ngtools/webpack                  8.0.4
@schematics/angular               8.0.4
@schematics/update                0.800.4
rxjs                              6.4.0
typescript                        3.4.5
webpack                           4.30.0

Anything else relevant?
Experienced in IE11 but the problem's a more general one.

@filipesilva
Copy link
Contributor

This is an interesting case. The short answer is "you need to manually include worker polyfills in the worker itself".

Workers are self-contained entry points. This means that their JS bundle does not share any of the packages of your application. That's why they have their own tsconfig.json as well. If you import a big package in your app and in your worker, it will be loaded twice. If you import a big package in your app and in two workers, it will be loaded three times.

They must be self contained because Web Worker run in background threads, so they need to contain all the code they need to run.

Nothing so far implies that we couldn't automatically add polyfills though. We already know what polyfills you want for your app, and we also have custom polyfills for es5 and jit. We're fairly sure the app will need the custom es5/jit polyfills because @angular-devkit/build-angular (the build system) is meant to build Angular apps.

But I don't think we should automatically add these to workers because polyfills can be fairly big and it's not very clear you will need them. Workers can be quite small if you're using them just to process data. The polyfills can be several times the size of a worker, and delay its startup. So I think the better approach for workers is to include polyfills they need directly.

Polyfills that are only loaded on older browsers are a harder topic. We do some tricks in index.html to convince the browser to only load some bundles on older browsers. But workers don't have a index.html and are just bundles instead. Maybe we could find a way to conditionally load polyfills on workers, but I'm not sure it's worth it. You'd probably need a custom polyfills file for each worker to make that work even if we had a way to do it.

@j2L4e
Copy link
Author

j2L4e commented Jul 10, 2019

Thanks for the thorough reply. I'm aware of the general concept (or problem) of the workers being self-contained.

The polyfills can be several times the size of a worker, and delay its startup.

That's the problem exactly. Including them manually is fine, but you end up loading them unnecessarily most of the time.

Can you think of a way to omit unneeded ones from the bundle rather than including needed ones?

@mgechev
Copy link
Member

mgechev commented Jul 11, 2019

Maybe a feature check and loading only the missing APIs could do the trick?

@j2L4e
Copy link
Author

j2L4e commented Jul 11, 2019

You'd still end up loading/parsing the whole bundle.
I see how this is an edge case though and adding them by hand seems managable.
Thanks for your time, guys! 👍

@filipesilva
Copy link
Contributor

I think at some point https://polyfill.io/ allowed you to make a call and it would give you what your browser lacked somehow. I'm not sure if it still does that. But the round trip might be worse than just including some polyfills directly.

@filipesilva filipesilva added the feature Issue that requests a new feature label Jul 11, 2019
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Jul 11, 2019
@j2L4e
Copy link
Author

j2L4e commented Jul 11, 2019

Didn't know that one! Nice.

@mgechev
Copy link
Member

mgechev commented Jul 11, 2019

@j2L4e, yes, my suggestion was to add them by hand after a feature check but polyfill.io would do the trick by performing a user-agent check.

Can we go ahead and close this issue?

@j2L4e
Copy link
Author

j2L4e commented Jul 12, 2019

Seems to be the only reasonable way. Go ahead 👍

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants