feat(@schematics/angular): add lazy module path fixer

[phenomnomnominal]

Apologies, I messed up my original PR trying to rebase stuff :/

@alexeagle could you please ping the appropriate people for help with the tests?

Sorry again!

[alan-agius4]

I will try to look into the test issue that you are having.

I am thinking it might be because tslint uses the actual file system. Also I am not entirely use that you can use a TypeScript tslint rule for testing.

Maybe this test can help you https://github.com/angular/angular-cli/blob/master/packages/angular_devkit/schematics/tasks/tslint-fix/executor_spec.ts

[alan-agius4]

@phenomnomnominal. I did a couple of changes locally to try to get this running but it seems that the actual rule for some reason is throwing an error

[ '/tmp/devkit-host-1555415483626-20046/lazy-route.ts' ]
The 'no-lazy-module-paths' rule threw an error in '/tmp/devkit-host-1555415483626-20046/lazy-route.ts':
SyntaxError: Expected " ", "#", "*", "+", ",", ".", ":", ":first-child", ":has(", ":last-child", ":matches(", ":not(", ":nth-child(", ":nth-last-child(", ">", "[", "~" or [^ [\],():#!=><~+.] but ")" found.

Would you mind if I push the changes to your branch to at least make the lint rule run in the specs? Alternatively, you can take a look at the above link and try to make the test run :)

[phenomnomnominal]

Not at all please do! I think I've spotted the bug in the rule - seems like I have an extra ) from when I split it to multiple lines to pass lint 😅

[phenomnomnominal]

Okay, I think I'm actually getting closer! I have the rule running in a test I think!

[alan-agius4]

@phenomnomnominal, just pushed.

Note for later, kindly squash the commits :)

[alan-agius4]

This LGTM.

@alexeagle PTAL

[filipesilva]

This migration also needs to add "experimentalImportFactories": true to angular.json to make the import syntax supported in VE.

[phenomnomnominal]

@filipesilva sure! Excuse my ignorance, but I guess that means a separate PR? Does the migration also need to modify the tsconfig.json file?

[filipesilva]

Heya @phenomnomnominal! Yes that would need a separate PR. Now that I think about it, you raise a good point: the tsconfig also needs to use "module": "esnext" actually, otherwise the import won't work at all when compiling.

I think what we really need is a full e2e test to ensure all these things are happening on update.

[phenomnomnominal]

Great, I will have a go at a PR 👍

[filipesilva]

Let me know if you need help with the e2e. It will probably be a very similar to update-1.7.ts, that tests updating from this project.

The project that you want to update should have a lazy module with the string syntax and an e2e, much like this.

Then you update the project, check that import() etc etc is there, run ng e2e and ng e2e --prod, and that makes us pretty sure the update went well.

[filipesilva]

@phenomnomnominal I had some time today and ended up doing the e2e (#14228) and removing the option overall (#14231).

[phenomnomnominal]

@filipesilva oh thank you so much! I had a go with the e2e test and got stuff running but couldn't figure out how to run it against the local version of the CLI... but either way, thanks!

And removing the experimental flag seems smart since we're forcing people to use it!

[angular-automatic-lock-bot]

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.}