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

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

Merged
merged 1 commit into from Mar 13, 2019

Conversation

Projects
None yet
5 participants
@ocombe
Copy link
Contributor

ocombe commented Mar 13, 2019

For runtime i18n with ivy we generate code for both Closure compiler (used at Google) and for external people that don't use Closure compiler.
We added a new global flag named ngI18nClosureMode that allows uglify (and Closure compiler) to effectively tree-shake all of the code that isn't used in the current setup.
By default we remove all of the Closure compiler code because we assume that it won't be used.

Required for angular/angular#28689.

feat(@angular-devkit/build-angular): remove Closure compiler i18n cod…
…e for ivy

For runtime i18n with ivy we generate code for both Closure compiler (used at Google) and for external people that don't use Closure compiler.
We added a new global flag named `ngI18nClosureMode` that allows uglify (and Closure compiler) to effectively tree-shake all of the code that isn't used in the current setup.
By default we remove all of the Closure compiler code because we assume that it won't be used.
compress: (buildOptions.platform == 'server' ? {
global_defs: {
ngDevMode: false,
ngI18nClosureMode: false,

This comment has been minimized.

@filipesilva

filipesilva Mar 13, 2019

Member

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

This comment has been minimized.

@ocombe

ocombe Mar 13, 2019

Author Contributor

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

This comment has been minimized.

@filipesilva

filipesilva Mar 13, 2019

Member

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?

This comment has been minimized.

@mhevery

mhevery Mar 13, 2019

Member

@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?

This comment has been minimized.

@filipesilva

filipesilva Mar 13, 2019

Member

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

compress: (buildOptions.platform == 'server' ? {
global_defs: {
ngDevMode: false,
ngI18nClosureMode: false,

This comment has been minimized.

@mhevery

mhevery Mar 13, 2019

Member

@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?

@vikerman vikerman merged commit 3d8064b into master Mar 13, 2019

12 of 15 checks passed

ci/angular: merge status Status "continuous-integration/appveyor/branch" is failing, status "buildkite/cli" is pending
continuous-integration/appveyor/branch AppVeyor build failed
Details
buildkite/cli Build #209 started
Details
ci/angular: size No baseline available for master / 3ac1cc30a7937784cda54f7762c5831fef6a9a59
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: build-bazel Your tests passed on CircleCI!
Details
ci/circleci: e2e-cli Your tests passed on CircleCI!
Details
ci/circleci: install Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: snapshot_publish Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
ci/circleci: test-large Your tests passed on CircleCI!
Details
ci/circleci: validate Your tests passed on CircleCI!
Details
cla/google All necessary CLAs are signed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@ocombe ocombe deleted the feat-closure-i18n-option branch Mar 14, 2019

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.