Skip to content

feat(@angular-devkit/build-angular): remove Closure compiler i18n code for ivy #13905

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.

Sign up for GitHub

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

Merged
vikerman merged 1 commit into from
Mar 13, 2019
Merged

feat(@angular-devkit/build-angular): remove Closure compiler i18n code for ivy #13905

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion packages/angular_devkit/build_angular/src/angular-cli-files/models/webpack-configs/common.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,10 +237,11 @@ export function getCommonConfig(wco: WebpackConfigOptions) {
},

// On server, we don't want to compress anything. We still set the ngDevMode = false for it
// to remove dev code.
// to remove dev code, and ngI18nClosureMode to remove Closure compiler i18n code
compress: (buildOptions.platform == 'server' ? {
global_defs: {
ngDevMode: false,
ngI18nClosureMode: false,

@filipesilva filipesilva Mar 13, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if this isn't added? Are bundles bigger or are they broken?

@ocombe ocombe Mar 13, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just bigger (it breaks the integration test on angular right now because the size diff is >1%, but other than that everything works)

@filipesilva filipesilva Mar 13, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For how long is this flag expected to be around? Is it part of the necessary tooling for Ivy in general, or is it expected to not be necessary after a certain version?

@mhevery mhevery Mar 13, 2019

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@filipesilva ngI18nClosureMode will be around for as long as we want to support goog.getMsg. The reason we need this is because we generate

let greeting;
if (ngI18nClosureMode) {
  const MSG_greeting = goog.getMsg('Hello');
} else {
  greeting = localize`Hello`;
}

We need to generate the above because we need to ship the code in the above format to NPM so that developers which download a 3P library can choose to compile it with closure or without.

Come to think of it both ngDevMode and ngI18nClosureMode need to be defined only for the final application but NOT for library compilations. We want to make sure that when we ship things into NPM ngDevMode and ngI18nClosureMode are preserved in the source code, because it is only the application developer who can make a choice if they are building a debug build or production and if they are using goog.getMsg or something else.

What is the current best practice for building an angular library? Would the above settings apply? If so can we have a different pass for libraries which do not set these values?

@filipesilva filipesilva Mar 13, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Libraries use a separate pipeline. This one is just for apps.

},
} : {
pure_getters: buildOptions.buildOptimizer,
Expand All @@ -249,6 +250,7 @@ export function getCommonConfig(wco: WebpackConfigOptions) {
passes: buildOptions.buildOptimizer ? 3 : 1,
global_defs: {
ngDevMode: false,
ngI18nClosureMode: false,
},
}),
// We also want to avoid mangling on server.
Expand Down