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

Add an option to remove reflect-metadata polyfills from prod bundle #6325

Closed
IgorMinar opened this issue May 15, 2017 · 8 comments
Closed

Add an option to remove reflect-metadata polyfills from prod bundle #6325

IgorMinar opened this issue May 15, 2017 · 8 comments
Labels
effort2: medium (days) feature Issue that requests a new feature P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent

Comments

@IgorMinar
Copy link
Contributor

Bug Report or Feature Request (mark with an x)

- [x] feature request

Versions.

@angular/cli: 1.0.3
node: 6.9.5
os: darwin x64
@angular/animations: 4.2.0-beta.1
@angular/common: 4.2.0-beta.1
@angular/compiler: 4.2.0-beta.1
@angular/core: 4.2.0-beta.1
@angular/forms: 4.2.0-beta.1
@angular/http: 4.2.0-beta.1
@angular/material: 2.0.0-beta.4
@angular/platform-browser: 4.2.0-beta.1
@angular/platform-browser-dynamic: 4.2.0-beta.1
@angular/platform-server: 4.2.0-beta.1
@angular/router: 4.2.0-beta.1
@angular/service-worker: 1.0.0-beta.13
@angular/cli: 1.0.3
@angular/compiler-cli: 4.2.0-beta.1

Desired functionality.

reflect-metadata is a sizible chunk of code that slows down the critical path in production mode. It's only needed in the JIT (dev) mode (however we want to drop the need for in from core as well).

Currently there is no way to include the polyfill only for dev bundles. I tried using environment.ts but that gets loaded too late after the reflect api is already used by the app code.

It would be great to have a way to either strip the polyfill or have an alternative entry path for production polyfills.

@sumitarora sumitarora self-assigned this May 16, 2017
@sumitarora sumitarora added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent feature Issue that requests a new feature effort2: medium (days) labels May 16, 2017
@IgorMinar
Copy link
Contributor Author

I ended up implementing this via the following hack:

diff --git a/aio/src/environments/environment.ts b/aio/src/environments/environment.ts
index 69c1f7c8bd2..165a718f559 100644
--- a/aio/src/environments/environment.ts
+++ b/aio/src/environments/environment.ts
@@ -3,6 +3,15 @@
 // `ng build --env=prod` then `environment.prod.ts` will be used instead.
 // The list of which env maps to which file can be found in `angular-cli.json`.
 
+
+// Reflect.metadata polyfill is only needed in the JIT/dev mode.
+//
+// In order to load these polyfills early enough (before app code), polyfill.ts imports this file to
+// to change the order in the final bundle.
+import 'core-js/es6/reflect';
+import 'core-js/es7/reflect';
+
+
 export const environment = {
   gaId: 'UA-8594346-26', // Staging site
   production: false
diff --git a/aio/src/polyfills.ts b/aio/src/polyfills.ts
index 53bdaf1b864..1e6698d2160 100644
--- a/aio/src/polyfills.ts
+++ b/aio/src/polyfills.ts
@@ -40,9 +40,8 @@
 // import 'web-animations-js';  // Run `npm install --save web-animations-js`.
 
 
-/** Evergreen browsers require these. **/
-import 'core-js/es6/reflect';
-import 'core-js/es7/reflect';
+/** HACK: force import of environment.ts/environment.prod.ts to load env specific polyfills */
+import './environments/environment';
 
 
 /** ALL Firefox browsers require the following to support `@angular/animation`. **/

works fine, but it's uglify and not obvious that this is what you should do to get rid of the polyfills.

@dominique-mueller
Copy link

While the solution above works pretty well, be aware that polyfills might end up twice in your bundles - one time in the polyfills bundle, and the second time in the vendor bundle (because environment is part of vendor).

I've discovered this using the webpack-bundle-analyzer, and fixed it by setting the polyfills attribute in the .angular-cli.json file to an empty string (meaning that, consequently, no polyfills bundle will be emitted now).

@dominique-mueller
Copy link

Instead of doing this via different files, I'm wondering if this could also be achieved using Webpack ...

@dominique-mueller
Copy link

dominique-mueller commented Aug 23, 2018

Seems that patching the @angular\cli\models\webpack-configs\production.js file with the following content works:

externals: {
  'core-js/es6/reflect': 'reflect',
  'core-js/es7/reflect': 'reflect'
},

Or even shorter:

externals: /^core-js\/(es6|es7)\/reflect$/,

Wrote a short NodeJS script which patches internal files on postinstall, similar to moment/moment#2416 (comment):

console.log( 'Modifying Angular CLI configuration ...' );
fs.readFile( './node_modules/@angular/cli/models/webpack-configs/production.js', 'utf-8', ( error, fileContent ) => {

  // Add core-js reflect patch
  console.log( '  -> Adding core-js reflect fix ...' );
  const reflectPolyfillsFix = `externals: /^core-js\\/(es6|es7)\\/reflect$/,`;
  if ( fileContent.indexOf( reflectPolyfillsFix ) === -1 ) {
    const uniqueContentToReplace = 'return {';
    fileContent = fileContent.replace( uniqueContentToReplace, `${ uniqueContentToReplace }\n        ${ reflectPolyfillsFix }` );
    console.log( '     Done.' )
  } else {
    console.log( '     Nothing to do.' );
  }

  // Write modified file
  fs.writeFile( './node_modules/@angular/cli/models/webpack-configs/production.js', fileContent, 'utf-8', ( error ) => {
    console.log( 'Done!' );
  } );

} );

@IgorMinar
Copy link
Contributor Author

this has been implemented in v7

@dominique-mueller
Copy link

Hooray! :)

@jeetparikh
Copy link

We are not using angular cli for build.. is there a way we can exclude these polyfills only for production using webpack?

@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 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
effort2: medium (days) feature Issue that requests a new feature P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants