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

Incorrect imports added for ngInjectableDef with Bazel #25644

Closed
jelbourn opened this Issue Aug 23, 2018 · 10 comments

Comments

@jelbourn
Copy link
Member

jelbourn commented Aug 23, 2018

Summary

When building Angular Material with Bazel (including Angular from source), incorrect import statements are being written for ngInjectableDef statements.

Steps to reproduce

  1. Clone https://github.com/jelbourn/material2/tree/build-angular-from-source
  2. Grab a local snapshot of Angular for debugging:
wget https://github.com/angular/angular/archive/4e36f0cd68dfc5e0b9f53f6555cbc49a38efdcf7.zip -O /tmp/ng.zip
(cd /tmp ; unzip ng.zip)
  1. bazel build src/cdk:npm_package
  2. Examine the file:
$(bazel info bazel-bin)/src/cdk/npm_package.es6/src/cdk/drag-drop/drag-drop-registry.js
  1. Note the imports:
import * as i1 from "@angular/cdk/overlay/public-api.ngfactory";
import * as i2 from "@angular/cdk/bidi/dir-document-token.ngfactory";
  1. and the ngInjectableDef statement:
DragDropRegistry.ngInjectableDef = i0.defineInjectable({ factory: function DragDropRegistry_Factory() { return new DragDropRegistry(i0.inject(i1.NgZone_64), i0.inject(i2.DOCUMENT_4)); }, token: DragDropRegistry, providedIn: "root" });

Both injected tokens (DOCUMENT and NgZone) are being imported from the wrong place.

cc @IgorMinar

@alxhub

This comment has been minimized.

Copy link
Contributor

alxhub commented Aug 23, 2018

I don't know if there is a good way around this.

In the Bazel environment currently, ngc is run in a manner which produces .ngfactory files, which are available between compilation units. ngc naturally and correctly assumes that it can depend on .ngfactory files from other compilation units in order to produce its output (indeed, this is how g3 works).

The Bazel rule which creates NPM packages, however, strips .ngfactory files before publishing. Libraries with Angular are usually built with ngc flags which disable factory generation entirely, and with dependencies that do not have factories available.

@alexeagle

This comment has been minimized.

Copy link
Contributor

alexeagle commented Aug 27, 2018

There are no npm package publishes involved here. It should look like Google3.

@jelbourn

This comment has been minimized.

Copy link
Member Author

jelbourn commented Aug 27, 2018

@alexeagle The difference is rollup getting involved here to create an npm package. In google3 it works fine because there's nothing wrong with referencing the ngfactory files.

@alexeagle

This comment has been minimized.

Copy link
Contributor

alexeagle commented Aug 28, 2018

@devversion

This comment has been minimized.

Copy link
Member

devversion commented Jan 20, 2019

Just for tracking of what I found so far. Some things are already clear, but I want to reference them with their corresponding code lines.

Currently whenever a factory file gets created, external imports needed for the factory file are imported and re-exported as part of the same factory file.

exportAs.forEach((entry) => {
ngFactoryCtx.statements.push(
o.variable(entry.exportAs).set(ngFactoryCtx.importExpr(entry.symbol)).toDeclStmt(null, [
o.StmtModifier.Exported
]));
});

This is done because the corresponding .ngsummary file declares these external imports (e.g. NgZone or DOCUMENT) as exported from that factory file.

Now if we run NGC again with the ngfactory and ngsummary files referenced and "generateCodeForLibraries": false, the summary is deserialized and whenever the actual source files for example depend on NgZone, the AotSummaryResolver resolves the import expression to the factory re-export.

getImportAs(staticSymbol: StaticSymbol): StaticSymbol {
staticSymbol.assertNoMembers();
return this.importAs.get(staticSymbol) !;
}

This could be avoided by:

  1. Using generateCodeForLibraries != false. This option is set to false within Bazel by default. (see code logic)
  2. Or passing through the original StaticSymbol for external import expressions.

Approach 2 explanation

Instead of outputting a summary file for a file X that declares external imports as exported within file X, we just pass through the original location of the external symbol.

This would mean that we don't need to re-export symbols that are already exported, and that output files don't incorrectly import symbols through a layer of indirection (e.g. through factory file). e.g. the summary file would look like that:

{
    "__symbol": 9,
    "name": "NgZone",
    "filePath": "angular_material/external/angular/packages/core/src/zone/ng_zone",
    "importAs": {
        "filePath": "C:/users/paul/_bazel_paul/kn4tsvyh/execroot/angular_material/bazel-out/x64_windows-fastbuild/bin/external/angular/packages/core/src/zone/ng_zone.d.ts",
        "name": "NgZone",
        "members": []
    }
},

Previously it looked like that:

    {
      "__symbol": 9,
      "name": "NgZone",
      "filePath": "angular_material/external/angular/packages/core/src/zone/ng_zone",
      "importAs": "NgZone_9"
    },

This change would sound pretty reasonable to me, and could be implemented easily without big efforts since there already is a similar logic: see here.


tl;dr: There seem to be ways to fix this issue quite easily. Approach (1) would potentially require a lot of refactoring of the Angular Bazel rules, and approach (2) might be a bit risky, but sounds more reasonable and should be quite easy to implement.

I'm not too sure if it's worth the effort considering that Ivy fixes this issue easily.

@alxhub

This comment has been minimized.

Copy link
Contributor

alxhub commented Jan 29, 2019

@devversion I think your understanding of summaries and how they work now matches (if not exceeds) my own.

One issue that I can think of:

This would mean that we don't need to re-export symbols that are already exported, and that output files don't incorrectly import symbols through a layer of indirection (e.g. through factory file).

This actually happens for a specific reason. In google3 we have this concept of "strict deps" - if you import symbol X from module M, then at the BUILD level you have to have a direct dependency on the package that builds M. If you don't, the build system gives you an error.

Angular, as it turns out, doesn't play nicely with this system in certain cases. For example, BrowserModule (in @angular/platform-browser) exports CommonModule (from @angular/common), meaning users of BrowserModule can use NgIf and NgFor in their applications.

If the user writes a template with an NgIf, the NgFactory for that template will need to instantiate NgIf somehow, which means it must import it from somewhere. But where? If it imports it from @angular/common, then the generated code is creating a dependency on @angular/common where there wasn't one before - the user didn't need to know anything about @angular/common to get to this point. If we generated that import, we could fail strict deps checking if the user didn't actually configure that dependency. This is actually how Angular should work, and how Ivy does work.

Instead, we did this crazy solution where factories/summaries re-export types, and we import things via these convoluted re-exports rather than directly from the source. That way, the user's factory from above can import NgIf via BrowserModule and its factory, and there's no strict deps violation - the user doesn't need a hard dependency on @angular/common for this to work.

Maybe the right answer here is for Blaze and Bazel to behave differently, with Blaze continuing to write and consume the re-exports, and Bazel having more simplified metadata/summaries with direct references? It's stretching my understanding of how this whole system is put together, but that seems like it could work to me.

@devversion

This comment has been minimized.

Copy link
Member

devversion commented Jan 29, 2019

@alxhub That's a good point. Having different behavior for Blaze and Bazel sounds reasonable.

I'm just not too sure if it's worth putting that much effort into ngc while Ivy would fix this issue magically. I can try working on some small concept, but I still consider the whole change as potentially risky.

@alxhub

This comment has been minimized.

Copy link
Contributor

alxhub commented Jan 31, 2019

@devversion I agree, though the risk is mitigated considerably if Blaze maintains the old behavior.

@devversion

This comment has been minimized.

Copy link
Member

devversion commented Feb 5, 2019

@alxhub That's reasonable. I'm just not too sure how Blaze currently invokes NGC, and how I can implement different behavior in that case. I assume G3 also uses the ng_module rule right?

@alxhub

This comment has been minimized.

Copy link
Contributor

alxhub commented Feb 6, 2019

@devversion correct. In #28523 I introduce a flag to the compiler to switch behavior between Bazel/Blaze, you could follow this same model.

devversion added a commit to devversion/angular that referenced this issue Feb 7, 2019

refactor(compiler): allow disabling external symbol factory reexports
Currently external static symbols which are referenced by AOT compiler
generated code will be re-exported in the corresponding factory files.

This way of handling the symbol resolution has been introduced in favor
of avoding dynamically generated module dependencies. This behavior
therefore avoids any strict dependency failures (as with Bazel)

Read more about a particular scenario here: angular#25644 (comment)

Now with `ngtsc`, this behavior is no longer valid since `ngtsc` keeps
introducing these module dependencies in order to properly reference
the external symbol from its original location. Similarly we should provide
a way to use the same behavior with `ngc` because the downside of using
the re-exported symbol resolution is that user-code transformations (e.g.
the `ngInjectableDef` metadata which is added to the user source code),
can resolve external symbols to previous factory re-exports. This is a critical
issue because it means that the actual JIT code references factory files in order
to access external symbols. This means that the generated output cannot shipped
to NPM without shipping the referenced factory files.

A specific example has been reported here: angular#25644 (comment)

devversion added a commit to devversion/angular that referenced this issue Feb 7, 2019

fix(bazel): do not use external symbol factory re-exports
When building Angular with Bazel, AOT is enabled by default and therefore factories, metadata and summary
files are generated besides the transpiled sources. This is problematic because `ng_module` targets that
depend on other `ng_module` targets can end up importing external static symbols from other factory
files. This means that the given `ng_module` can only be published with the referenced factory files and
cannot be built with the `ng_package` rule since `ng_package` filters out all genfiles.

Fixes angular#25644.

devversion added a commit to devversion/angular that referenced this issue Feb 7, 2019

refactor(compiler): allow disabling external symbol factory reexports
Currently external static symbols which are referenced by AOT compiler
generated code will be re-exported in the corresponding factory files.

This way of handling the symbol resolution has been introduced in favor
of avoding dynamically generated module dependencies. This behavior
therefore avoids any strict dependency failures (as with Bazel)

Read more about a particular scenario here: angular#25644 (comment)

Now with `ngtsc`, this behavior is no longer valid since `ngtsc` keeps
introducing these module dependencies in order to properly reference
the external symbol from its original location. Similarly we should provide
a way to use the same behavior with `ngc` because the downside of using
the re-exported symbol resolution is that user-code transformations (e.g.
the `ngInjectableDef` metadata which is added to the user source code),
can resolve external symbols to previous factory re-exports. This is a critical
issue because it means that the actual JIT code references factory files in order
to access external symbols. This means that the generated output cannot shipped
to NPM without shipping the referenced factory files.

A specific example has been reported here: angular#25644 (comment)

devversion added a commit to devversion/angular that referenced this issue Feb 7, 2019

fix(bazel): do not use external symbol factory re-exports
When building Angular with Bazel, AOT is enabled by default and therefore factories, metadata and summary
files are generated besides the transpiled sources. This is problematic because `ng_module` targets that
depend on other `ng_module` targets can end up importing external static symbols from other factory
files. This means that the given `ng_module` can only be published with the referenced factory files and
cannot be built with the `ng_package` rule since `ng_package` filters out all genfiles.

Fixes angular#25644.

@devversion devversion self-assigned this Feb 7, 2019

devversion added a commit to devversion/angular that referenced this issue Feb 7, 2019

refactor(compiler): allow disabling external symbol factory reexports
Currently external static symbols which are referenced by AOT
compiler generated code, will be re-exported in the corresponding
`.ngfactory` files.

This way of handling the symbol resolution has been introduced in
favor of avoding dynamically generated module dependencies. This
behavior therefore avoids any strict dependency failures.

Read more about a particular scenario here: angular#25644 (comment)

Now with `ngtsc`, this behavior has changed since `ngtsc` just
introduces these module dependencies in order to properly reference
the external symbol from its original location (also eliminating the need
for factories). Similarly we should provide a way to use the same
behavior with `ngc` because the downside of using the re-exported symbol
resolution is that user-code transformations (e.g. the `ngInjectableDef`
metadata which is added to the user source code), can resolve external
symbols to previous factory symbol re-exports. This is a critical issue
because it means that the actual JIT code references factory files in order
to access external symbols. This means that the generated output cannot
shipped to NPM without shipping the referenced factory files.

A specific example has been reported here: angular#25644 (comment)

devversion added a commit to devversion/angular that referenced this issue Feb 7, 2019

fix(bazel): do not use external symbol factory re-exports
When building Angular with Bazel, AOT is enabled by default and
therefore factories, metadata and summary files are generated besides
the transpiled sources. This is problematic because `ng_module` targets
that depend on other `ng_module` targets can end up importing external
static symbols from other factory files. This means that the given
`ng_module` can only be published with the referenced factory files
and cannot be built with the `ng_package` rule since `ng_package`
filters out all genfiles.

Fixes angular#25644.

devversion added a commit to devversion/angular that referenced this issue Feb 8, 2019

refactor(compiler): allow disabling external symbol factory reexports
Currently external static symbols which are referenced by AOT
compiler generated code, will be re-exported in the corresponding
`.ngfactory` files.

This way of handling the symbol resolution has been introduced in
favor of avoding dynamically generated module dependencies. This
behavior therefore avoids any strict dependency failures.

Read more about a particular scenario here: angular#25644 (comment)

Now with `ngtsc`, this behavior has changed since `ngtsc` just
introduces these module dependencies in order to properly reference
the external symbol from its original location (also eliminating the need
for factories). Similarly we should provide a way to use the same
behavior with `ngc` because the downside of using the re-exported symbol
resolution is that user-code transformations (e.g. the `ngInjectableDef`
metadata which is added to the user source code), can resolve external
symbols to previous factory symbol re-exports. This is a critical issue
because it means that the actual JIT code references factory files in order
to access external symbols. This means that the generated output cannot
shipped to NPM without shipping the referenced factory files.

A specific example has been reported here: angular#25644 (comment)

devversion added a commit to devversion/angular that referenced this issue Feb 8, 2019

fix(bazel): do not use external symbol factory re-exports
When building Angular with Bazel, AOT is enabled by default and
therefore factories, metadata and summary files are generated besides
the transpiled sources. This is problematic because `ng_module` targets
that depend on other `ng_module` targets can end up importing external
static symbols from other factory files. This means that the given
`ng_module` can only be published with the referenced factory files
and cannot be built with the `ng_package` rule since `ng_package`
filters out all genfiles.

Fixes angular#25644.

devversion added a commit to devversion/angular that referenced this issue Feb 8, 2019

refactor(compiler): allow disabling external symbol factory reexports
Currently external static symbols which are referenced by AOT
compiler generated code, will be re-exported in the corresponding
`.ngfactory` files.

This way of handling the symbol resolution has been introduced in
favor of avoding dynamically generated module dependencies. This
behavior therefore avoids any strict dependency failures.

Read more about a particular scenario here: angular#25644 (comment)

Now with `ngtsc`, this behavior has changed since `ngtsc` just
introduces these module dependencies in order to properly reference
the external symbol from its original location (also eliminating the need
for factories). Similarly we should provide a way to use the same
behavior with `ngc` because the downside of using the re-exported symbol
resolution is that user-code transformations (e.g. the `ngInjectableDef`
metadata which is added to the user source code), can resolve external
symbols to previous factory symbol re-exports. This is a critical issue
because it means that the actual JIT code references factory files in order
to access external symbols. This means that the generated output cannot
shipped to NPM without shipping the referenced factory files.

A specific example has been reported here: angular#25644 (comment)

devversion added a commit to devversion/angular that referenced this issue Feb 8, 2019

fix(bazel): do not use external symbol factory re-exports
When building Angular with Bazel, AOT is enabled by default and
therefore factories, metadata and summary files are generated besides
the transpiled sources. This is problematic because `ng_module` targets
that depend on other `ng_module` targets can end up importing external
static symbols from other factory files. This means that the given
`ng_module` can only be published with the referenced factory files
and cannot be built with the `ng_package` rule since `ng_package`
filters out all genfiles.

Fixes angular#25644.

devversion added a commit to devversion/angular that referenced this issue Feb 9, 2019

refactor(compiler): allow disabling external symbol factory reexports
Currently external static symbols which are referenced by AOT
compiler generated code, will be re-exported in the corresponding
`.ngfactory` files.

This way of handling the symbol resolution has been introduced in
favor of avoding dynamically generated module dependencies. This
behavior therefore avoids any strict dependency failures.

Read more about a particular scenario here: angular#25644 (comment)

Now with `ngtsc`, this behavior has changed since `ngtsc` just
introduces these module dependencies in order to properly reference
the external symbol from its original location (also eliminating the need
for factories). Similarly we should provide a way to use the same
behavior with `ngc` because the downside of using the re-exported symbol
resolution is that user-code transformations (e.g. the `ngInjectableDef`
metadata which is added to the user source code), can resolve external
symbols to previous factory symbol re-exports. This is a critical issue
because it means that the actual JIT code references factory files in order
to access external symbols. This means that the generated output cannot
shipped to NPM without shipping the referenced factory files.

A specific example has been reported here: angular#25644 (comment)

devversion added a commit to devversion/angular that referenced this issue Feb 9, 2019

feat(compiler-cli): no longer re-export external symbols by default
With angular#28594 we refactored the `@angular/compiler` slightly to
allow opting out from external symbol re-exports which are
enabled by default.

Since symbol re-exports only benefit projects which have a
very strict dependency enforcement, external symbols should
not be re-exported by default as this could grow the size of
factory files and cause unexpected behavior with Angular's
AOT symbol resolving (e.g. see: angular#25644).

Additionally "ngtsc" also does not provide any way of using
external symbol re-exports, so this means that NGC partially
matches the behavior of "ngtsc" then (unless explicitly opted-out).

Internally at Google, symbol re-exports need to be still enabled
since Google has a very strict dependency enforcement (even of
produced JavaScript output), so the `ng_module` Bazel rule will
enable the symbol re-exports by default when running within Blaze.

Fixes angular#25644.

devversion added a commit to devversion/angular that referenced this issue Feb 9, 2019

feat(compiler-cli): no longer re-export external symbols by default
With angular#28594 we refactored the `@angular/compiler` slightly to
allow opting out from external symbol re-exports which are
enabled by default.

Since symbol re-exports only benefit projects which have a
very strict dependency enforcement, external symbols should
not be re-exported by default as this could grow the size of
factory files and cause unexpected behavior with Angular's
AOT symbol resolving (e.g. see: angular#25644).

Additionally "ngtsc" also does not provide any way of using
external symbol re-exports, so this means that NGC partially
matches the behavior of "ngtsc" then (unless explicitly opted-out).

Internally at Google, symbol re-exports need to be still enabled
since Google has a very strict dependency enforcement (even of
produced JavaScript output), so the `ng_module` Bazel rule will
enable the symbol re-exports by default when running within Blaze.

Fixes angular#25644.

devversion added a commit to devversion/angular that referenced this issue Feb 9, 2019

feat(compiler-cli): no longer re-export external symbols by default
With angular#28594 we refactored the `@angular/compiler` slightly to
allow opting out from external symbol re-exports which are
enabled by default.

Since symbol re-exports only benefit projects which have a
very strict dependency enforcement, external symbols should
not be re-exported by default as this could grow the size of
factory files and cause unexpected behavior with Angular's
AOT symbol resolving (e.g. see: angular#25644).

Additionally "ngtsc" also does not provide any way of using
external symbol re-exports, so this means that NGC partially
matches the behavior of "ngtsc" then (unless explicitly opted-out).

Internally at Google, symbol re-exports need to be still enabled
since Google has a very strict dependency enforcement (even of
produced JavaScript output), so the `ng_module` Bazel rule will
enable the symbol re-exports by default when running within Blaze.

Fixes angular#25644.

devversion added a commit to devversion/angular that referenced this issue Feb 10, 2019

feat(compiler-cli): no longer re-export external symbols by default
With angular#28594 we refactored the `@angular/compiler` slightly to
allow opting out from external symbol re-exports which are
enabled by default.

Since symbol re-exports only benefit projects which have a
very strict dependency enforcement, external symbols should
not be re-exported by default as this could grow the size of
factory files and cause unexpected behavior with Angular's
AOT symbol resolving (e.g. see: angular#25644).

Note that the common strict dependency enforcement for source
files does still work with external symbol re-exports disabled,
but there are also strict dependency checks that enforce strict
module dependencies also for _generated files_ (such as the
ngfactory files). This is how Google3 manages it's dependencies
and therefore external symbol re-exports need to be enabled within
Google3.

Also "ngtsc" also does not provide any way of using external symbol
re-exports, so this means that with this change, NGC can partially
match the behavior of "ngtsc" then (unless explicitly opted-out).

As mentioned before, internally at Google symbol re-exports need to
be still enabled, so the `ng_module` Bazel rule will enable the symbol
re-exports by default when running within Blaze.

Fixes angular#25644.

mhevery added a commit that referenced this issue Feb 11, 2019

refactor(compiler): allow disabling external symbol factory reexports (
…#28594)

Currently external static symbols which are referenced by AOT
compiler generated code, will be re-exported in the corresponding
`.ngfactory` files.

This way of handling the symbol resolution has been introduced in
favor of avoding dynamically generated module dependencies. This
behavior therefore avoids any strict dependency failures.

Read more about a particular scenario here: #25644 (comment)

Now with `ngtsc`, this behavior has changed since `ngtsc` just
introduces these module dependencies in order to properly reference
the external symbol from its original location (also eliminating the need
for factories). Similarly we should provide a way to use the same
behavior with `ngc` because the downside of using the re-exported symbol
resolution is that user-code transformations (e.g. the `ngInjectableDef`
metadata which is added to the user source code), can resolve external
symbols to previous factory symbol re-exports. This is a critical issue
because it means that the actual JIT code references factory files in order
to access external symbols. This means that the generated output cannot
shipped to NPM without shipping the referenced factory files.

A specific example has been reported here: #25644 (comment)

PR Close #28594

mhevery added a commit that referenced this issue Feb 11, 2019

refactor(compiler): allow disabling external symbol factory reexports (
…#28594)

Currently external static symbols which are referenced by AOT
compiler generated code, will be re-exported in the corresponding
`.ngfactory` files.

This way of handling the symbol resolution has been introduced in
favor of avoding dynamically generated module dependencies. This
behavior therefore avoids any strict dependency failures.

Read more about a particular scenario here: #25644 (comment)

Now with `ngtsc`, this behavior has changed since `ngtsc` just
introduces these module dependencies in order to properly reference
the external symbol from its original location (also eliminating the need
for factories). Similarly we should provide a way to use the same
behavior with `ngc` because the downside of using the re-exported symbol
resolution is that user-code transformations (e.g. the `ngInjectableDef`
metadata which is added to the user source code), can resolve external
symbols to previous factory symbol re-exports. This is a critical issue
because it means that the actual JIT code references factory files in order
to access external symbols. This means that the generated output cannot
shipped to NPM without shipping the referenced factory files.

A specific example has been reported here: #25644 (comment)

PR Close #28594

devversion added a commit to devversion/angular that referenced this issue Feb 11, 2019

feat(compiler-cli): no longer re-export external symbols by default
With angular#28594 we refactored the `@angular/compiler` slightly to
allow opting out from external symbol re-exports which are
enabled by default.

Since symbol re-exports only benefit projects which have a
very strict dependency enforcement, external symbols should
not be re-exported by default as this could grow the size of
factory files and cause unexpected behavior with Angular's
AOT symbol resolving (e.g. see: angular#25644).

Additionally "ngtsc" also does not provide any way of using
external symbol re-exports, so this means that NGC partially
matches the behavior of "ngtsc" then (unless explicitly opted-out).

Internally at Google, symbol re-exports need to be still enabled
since Google has a very strict dependency enforcement (even of
produced JavaScript output), so the `ng_module` Bazel rule will
enable the symbol re-exports by default when running within Blaze.

Fixes angular#25644.

devversion added a commit to devversion/angular that referenced this issue Feb 12, 2019

feat(compiler-cli): no longer re-export external symbols by default
With angular#28594 we refactored the `@angular/compiler` slightly to
allow opting out from external symbol re-exports which are
enabled by default.

Since symbol re-exports only benefit projects which have a
very strict dependency enforcement, external symbols should
not be re-exported by default as this could grow the size of
factory files and cause unexpected behavior with Angular's
AOT symbol resolving (e.g. see: angular#25644).

Additionally "ngtsc" also does not provide any way of using
external symbol re-exports, so this means that NGC partially
matches the behavior of "ngtsc" then (unless explicitly opted-out).

Internally at Google, symbol re-exports need to be still enabled
since Google has a very strict dependency enforcement (even of
produced JavaScript output), so the `ng_module` Bazel rule will
enable the symbol re-exports by default when running within Blaze.

Fixes angular#25644.

devversion added a commit to devversion/angular that referenced this issue Feb 12, 2019

feat(compiler-cli): no longer re-export external symbols by default
With angular#28594 we refactored the `@angular/compiler` slightly to
allow opting out from external symbol re-exports which are
enabled by default.

Since symbol re-exports only benefit projects which have a
very strict dependency enforcement, external symbols should
not be re-exported by default as this could grow the size of
factory files and cause unexpected behavior with Angular's
AOT symbol resolving (e.g. see: angular#25644).

Additionally "ngtsc" also does not provide any way of using
external symbol re-exports, so this means that NGC partially
matches the behavior of "ngtsc" then (unless explicitly opted-out).

Internally at Google, symbol re-exports need to be still enabled
since Google has a very strict dependency enforcement (even of
produced JavaScript output), so the `ng_module` Bazel rule will
enable the symbol re-exports by default when running within Blaze.

Fixes angular#25644.

devversion added a commit to devversion/angular that referenced this issue Feb 12, 2019

feat(compiler-cli): no longer re-export external symbols by default
With angular#28594 we refactored the `@angular/compiler` slightly to
allow opting out from external symbol re-exports which are
enabled by default.

Since symbol re-exports only benefit projects which have a
very strict dependency enforcement, external symbols should
not be re-exported by default as this could grow the size of
factory files and cause unexpected behavior with Angular's
AOT symbol resolving (e.g. see: angular#25644).

Note that the common strict dependency enforcement for source
files does still work with external symbol re-exports disabled,
but there are also strict dependency checks that enforce strict
module dependencies also for _generated files_ (such as the
ngfactory files). This is how Google3 manages it's dependencies
and therefore external symbol re-exports need to be enabled within
Google3.

Also "ngtsc" also does not provide any way of using external symbol
re-exports, so this means that with this change, NGC can partially
match the behavior of "ngtsc" then (unless explicitly opted-out).

As mentioned before, internally at Google symbol re-exports need to
be still enabled, so the `ng_module` Bazel rule will enable the symbol
re-exports by default when running within Blaze.

Fixes angular#25644.

@mhevery mhevery closed this in 91b7152 Feb 13, 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.