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

fix(@ngtools/webpack): fix aot builds using npm packages that have la… #5594

Closed
wants to merge 2 commits into from

Conversation

maleetz
Copy link
Contributor

@maleetz maleetz commented Mar 23, 2017

Currently, code splitting derived from routes with the loadChildren property only works
if the resolved source file is a *.ts file in the source directory of your angular-cli
project. If your project uses an npm package that has a route with loadChildren, the
build will resolve the lazy loaded module as a *.d.ts definition file and then attempt
to find an *.d.ts.ngfactory.ts file in the generated AOT directory, which does not exist.
This fixes the path generation logic so the build will look for a *.ngfactory.ts file
in $$_gendir/node_modules.

const lr = path.relative(this.basePath, lazyRoute.replace(/\.ts$/, '.ngfactory.ts'));
this._lazyRoutes[k + '.ngfactory'] = path.join(this.genDir, lr);
const lr = path.relative(this.basePath, lazyRoute.replace(/(\.d)?\.ts$/, '.ngfactory.ts'));
this._lazyRoutes[k + '.ngfactory'] = path.join(this.genDir, path.join('/', lr));
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand the change on the line before (which LGTM), but I'm curious for why do you join / here and what does that mean for Windows users?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the comment! I'm thinking I should have used path.sep instead to be platform agnostic. Will update the PR shortly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And the purpose of the join is to ensure lr cannot traverse up the directory tree and escape the $$_gendir directory. Assuming the following:

  • basePath: '/project/src'
  • genDir: '/project/src/$$_gendir
  • lazyRoute: '/project/node_modules/package/lazy.module.ngfactory.ts'.

lr would then be '../node_modules/package/lazy.module.ngfactory.ts' and then the resulting _lazyRoutes path would end up being '/project/src/node_modules/package/lazy.module.ngfactory.ts', but the actual AOT factory file would be in '/project/src/$$_gendir/node_modules/package/lazy.module.ngfactory.ts'

@maleetz maleetz force-pushed the fix-lazy-load-npm-package branch from 1226c36 to b5b024c Compare March 23, 2017 18:13
@filipesilva
Copy link
Contributor

@hansl can you re-review?

…zy loaded modules

Currently, code splitting derived from routes with the loadChildren property only works
if the resolved source file is a *.ts file in the source directory of your angular-cli
project. If your project uses an npm package that has a route with loadChildren, the
build will resolve the lazy loaded module as a *.d.ts definition file and then attempt
to find an *.d.ts.ngfactory.ts file in the generated AOT directory, which does not exist.
This fixes the path generation logic so the build will look for a *.ngfactory.ts file
in $$_gendir/node_modules.
@maleetz maleetz force-pushed the fix-lazy-load-npm-package branch from b5b024c to 18ee60b Compare April 21, 2017 20:14
@maleetz
Copy link
Contributor Author

maleetz commented Apr 21, 2017

Fixed the linter issue but I couldn't reproduce the issue from e2e test case tests/build/rebuild.

@maleetz maleetz closed this May 5, 2017
@maleetz maleetz reopened this May 5, 2017
@maleetz
Copy link
Contributor Author

maleetz commented May 5, 2017

Any suggestions on what I need to do to avoid this test failure? It seems it is examining commits that it probably shouldn't.

Examining 1331 commit(s) between HEAD and master
INVALID COMMIT MSG: "Initial commit"
=> ERROR: The commit message does not match the format of "(): OR revert: type(): "

@hansl hansl closed this in 8465b29 May 24, 2017
Brocco pushed a commit that referenced this pull request May 24, 2017
…zy loaded modules

Currently, code splitting derived from routes with the loadChildren property only works
if the resolved source file is a *.ts file in the source directory of your angular-cli
project. If your project uses an npm package that has a route with loadChildren, the
build will resolve the lazy loaded module as a *.d.ts definition file and then attempt
to find an *.d.ts.ngfactory.ts file in the generated AOT directory, which does not exist.
This fixes the path generation logic so the build will look for a *.ngfactory.ts file
in $$_gendir/node_modules.

Closes #5594
Brocco pushed a commit that referenced this pull request May 24, 2017
…zy loaded modules

Currently, code splitting derived from routes with the loadChildren property only works
if the resolved source file is a *.ts file in the source directory of your angular-cli
project. If your project uses an npm package that has a route with loadChildren, the
build will resolve the lazy loaded module as a *.d.ts definition file and then attempt
to find an *.d.ts.ngfactory.ts file in the generated AOT directory, which does not exist.
This fixes the path generation logic so the build will look for a *.ngfactory.ts file
in $$_gendir/node_modules.

Closes #5594
dond2clouds pushed a commit to d2clouds/speedray-cli that referenced this pull request Apr 23, 2018
…zy loaded modules

Currently, code splitting derived from routes with the loadChildren property only works
if the resolved source file is a *.ts file in the source directory of your angular-cli
project. If your project uses an npm package that has a route with loadChildren, the
build will resolve the lazy loaded module as a *.d.ts definition file and then attempt
to find an *.d.ts.ngfactory.ts file in the generated AOT directory, which does not exist.
This fixes the path generation logic so the build will look for a *.ngfactory.ts file
in $$_gendir/node_modules.

Closes angular#5594
@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 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants