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

Not possible to generate a correct ngfactory import for an npm package using summary files #29454

Closed
alexeagle opened this issue Mar 22, 2019 · 2 comments
Labels
area: bazel Issues related to the published `@angular/bazel` build rules
Milestone

Comments

@alexeagle
Copy link
Contributor

Here is the scenario that Bazel users will encounter:

  1. Libraries like Material come from npm and are published with .metadata.json files rather than codegen outputs (at least until a later point when the codegen is stable and we recommend that libraries publish it, maybe around v9)
  2. Bazel rules communicate symbol information with each other with .ngsummary.json files, partly to avoid cascading re-builds. They cannot do codegen for npm packages since that would mean two rules have conflicting outputs, or duplicate the code
  3. To bridge the two worlds, we run ngc as a postinstall step so that npm packages contain codegen before Bazel references them. This creates ngsummary.json files from metadata.json files, though @alxhub is surprised that it even works to do that.
  4. The translation of .metadata.json to .ngsummary.json files works okay for a leaf compilation.

However, a downstream compilation will read from the intermediate ngsummary.json file and will produce undeclared symbols in the .ngfactory.js output.

For example, in angular-bazel-example
https://github.com/angular/angular-bazel-example/tree/c1f9819a9cc5f431a970a4ab6125860eef32b5a0

If we build //src/material we get

dist/bin/src/material/material.module.ngsummary.js

    const i16 = require("@angular/material/toolbar");
...
{ reference: i16.MatToolbar }, 
...

which is good, but the corresponding .json output doesn't propagate the module name:

dist/bin/src/material/material.module.ngsummary.json

{
    "moduleName": "angular_bazel_example/src/material/material.module",
    "symbols": [
        {
            "__symbol": 66,
            "name": "MatToolbar",
            "filePath": "angular_bazel_example/external/npm/node_modules/@angular/material/toolbar/typings/index",
            "moduleName":
        },

This means that a downstream compilation unit like //src will produce
dist/bin/src/application.module.ngfactory.js that references the MatToolbar symbol but never declares it, leading to runtime errors.

Here's an analysis of how we get to that state:

First, within the compiler, we don't find an import location for this. We try to write the import to the ngfactory here:

.toDeclStmt(o.expressionType(outputCtx.importExpr(

which calls to the _fileNameToModuleName
return this._summaryResolver.getKnownModuleName(importedFilePath) ||

but the importedFilePath is not found in the summaryResolver's knownModuleName lookup table.

Then we fall through to asking the host. Even if we strip off the external/npm/node_modules, the host cannot know the right import because the javascript file to load is in a different place from the files we read the summary data.

FAQ

  1. Why don't non-bazel users have this problem? They use the MetadataResolver which always gets the moduleName. The summaryResolver is used only under bazel/blaze
  2. Why don't blaze users have this problem? They build everything from source (as we've done under Bazel as well until recently) and so they only rely on summaries, not on metadata.
@gregmagolan gregmagolan added the area: bazel Issues related to the published `@angular/bazel` build rules label Mar 23, 2019
@ngbot ngbot bot added this to the needsTriage milestone Mar 23, 2019
@alexeagle
Copy link
Contributor Author

Was fixed by #29459

@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 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: bazel Issues related to the published `@angular/bazel` build rules
Projects
None yet
Development

No branches or pull requests

2 participants