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

fix(polyfills): move polyfills to own entry point #3812

Merged
merged 6 commits into from
Jan 20, 2017

Conversation

filipesilva
Copy link
Contributor

@filipesilva filipesilva commented Dec 31, 2016

Polyfills found in polyfills.ts would not be available for scripts due to being loaded in the main bundle only.

This PR loads polyfills as a separate entry point, before everything else.

Extra documentation also added to polyfills.ts.

Fix #2752
Fix #3309
Fix #4140

@filipesilva filipesilva force-pushed the move-polyfills branch 5 times, most recently from e1506f0 to e289a55 Compare January 3, 2017 00:59
@cebor
Copy link
Contributor

cebor commented Jan 7, 2017

Webpack also accepts modules/package names as entry option.

entry: {
  polyfills: [
    'core-js/es6',
     'core-js/es7/reflect',
     'zone.js/dist/zone'
  ],
...
}

Would not this be a cleaner solution?

@filipesilva
Copy link
Contributor Author

@cebor it's true that webpack accepts that, but the purpose of the scripts array is to provide feature parity with script tags. Script tags don't accept package names. I suppose the feature could be enhanced but it doesn't seem too important.

@hansl
Copy link
Contributor

hansl commented Jan 18, 2017

(cf. #3999)

While you’re at refactoring the polyfills and how they’re included, could you include the long stack trace when using debug? Would fix #2233.

@filipesilva filipesilva force-pushed the move-polyfills branch 3 times, most recently from 69426a6 to c2a2ea9 Compare January 20, 2017 18:37
@filipesilva
Copy link
Contributor Author

@hansl see #2233 (comment), adding the long stack trace is not the solution to that issue.

@filipesilva
Copy link
Contributor Author

Also need to add a lot of comments to polyfills to show opt-in ones for each browser.

Polyfills found in polyfills.ts would not be available for scripts due to being loaded in the main bundle only.

Fix angular#2752
Fix angular#3309
@filipesilva filipesilva changed the title fix(polyfills): move polyfills to scripts array fix(polyfills): move polyfills to own entry point Jan 20, 2017
@hansl
Copy link
Contributor

hansl commented Jan 20, 2017

LGTM, Thanks!

@filipesilva filipesilva merged commit 08bb738 into angular:master Jan 20, 2017
@filipesilva filipesilva deleted the move-polyfills branch January 20, 2017 23:52
MRHarrison pushed a commit to MRHarrison/angular-cli that referenced this pull request Feb 9, 2017
Polyfills found in polyfills.ts would not be available for scripts due to being loaded in the main bundle only.

This PR loads polyfills as a separate entry point, before everything else.

Extra documentation also added to polyfills.ts.

Fix angular#2752
Fix angular#3309
Fix angular#4140
@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 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants