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(router): deprecate loadChildren:string #30073

Closed

Conversation

alxhub
Copy link
Member

@alxhub alxhub commented Apr 23, 2019

ES2017 dynamic import() is now supported by the Angular CLI and the larger
toolchain. This renders the loadChildren: string API largely redundant, as
import() is far more natural, is less error-prone, and is standards
compliant. This commit deprecates the string form of loadChildren in
favor of dynamic import().

@alxhub alxhub requested a review from a team as a code owner April 23, 2019 21:45
@alxhub
Copy link
Member Author

alxhub commented Apr 23, 2019

Reviewer: please also review the commit message deprecation section, as it's intended to go in the CHANGELOG for 8.0.

@ngbot ngbot bot added this to the needsTriage milestone Apr 23, 2019
@alxhub alxhub force-pushed the router/loadChildren/deprecation branch from e46ef67 to e0d2c35 Compare April 23, 2019 21:54
@alxhub alxhub requested a review from a team as a code owner April 23, 2019 21:54
@alxhub alxhub requested a review from kara April 23, 2019 21:54
@alxhub alxhub added action: merge The PR is ready for merge by the caretaker target: major This PR is targeted for the next major release labels Apr 23, 2019
@alxhub
Copy link
Member Author

alxhub commented Apr 23, 2019

Presubmit

@@ -93,6 +93,16 @@ export type ResolveData = {
/**
*
* A function that is called to resolve a collection of lazy-loaded routes.
*
* Often this function will be implemented using an ES2017 dynamic `import()` expression. For
Copy link
Contributor

Choose a reason for hiding this comment

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

Dynamic import is not part of ES2017 (or any edition), only a stage-3 proposal.

Copy link
Member Author

Choose a reason for hiding this comment

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

Corrected, good catch!

@trotyl
Copy link
Contributor

trotyl commented Apr 24, 2019

Shouldn't NgModuleFactoryLoader/SystemJsNgModuleLoader be deprecated as well? Which are dedicated for string children.

@alxhub
Copy link
Member Author

alxhub commented Apr 24, 2019

@trotyl indeed. I added them.

The proposed ES dynamic import() is now supported by the Angular CLI and the
larger toolchain. This renders the `loadChildren: string` API largely
redundant, as import() is far more natural, is less error-prone, and is
standards compliant. This commit deprecates the `string` form of
`loadChildren` in favor of dynamic import().

DEPRECATION:

When defining lazy-loaded route, Angular previously offered two options for
configuring the module to be loaded, both via the `loadChildren` parameter
of the route. Most Angular developers are familiar withthe `string` form of
this API. For example, the following route definition configures Angular to
load a `LazyModule` NgModule from `lazy-route/lazy.module.ts`:

```
[{
  path: 'lazy',
  loadChildren: 'lazy-route/lazy.module#LazyModule',
}]
```

This "magic string" configuration was previously necessary as there was
no dynamic module loading standard on the web. This has changed with the
pending standardization of dynamic `import()` expressions, which are now
supported in the Angular CLI and in web tooling in general. `import()`
offers a more natural and robust solution to dynamic module loading. The
above example can be rewritten to use dynamic `import()`:

```
[{
  path: 'lazy',
  loadChildren: () => import('./lazy-route/lazy.module').then(mod => mod.LazyModule),
}]
```

This form of lazy loading offers significant advantages in terms of:

* type checking via TypeScript
* simplicity of generated code
* future potential to run natively in supporting browsers
  (see: [caniuse: dynamic import()](https://caniuse.com/#feat=es6-module-dynamic-import))

As a result, Angular is deprecating the `loadChildren: string` syntax in
favor of ES dynamic `import()`. An automatic migration will run during
`ng upgrade` to convert your existing Angular code to the new syntax.
@alxhub alxhub force-pushed the router/loadChildren/deprecation branch from 777f9c4 to 898e253 Compare April 24, 2019 22:30
Copy link
Contributor

@IgorMinar IgorMinar left a comment

Choose a reason for hiding this comment

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

looks good! Thanks for all the updates

@IgorMinar IgorMinar removed the request for review from kara April 24, 2019 23:46
@IgorMinar IgorMinar added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Apr 24, 2019
@IgorMinar
Copy link
Contributor

merge-assistance: global approval

@IgorMinar IgorMinar modified the milestones: needsTriage, version 8 Apr 24, 2019
BioPhoton pushed a commit to BioPhoton/angular that referenced this pull request May 21, 2019
The proposed ES dynamic import() is now supported by the Angular CLI and the
larger toolchain. This renders the `loadChildren: string` API largely
redundant, as import() is far more natural, is less error-prone, and is
standards compliant. This commit deprecates the `string` form of
`loadChildren` in favor of dynamic import().

DEPRECATION:

When defining lazy-loaded route, Angular previously offered two options for
configuring the module to be loaded, both via the `loadChildren` parameter
of the route. Most Angular developers are familiar withthe `string` form of
this API. For example, the following route definition configures Angular to
load a `LazyModule` NgModule from `lazy-route/lazy.module.ts`:

```
[{
  path: 'lazy',
  loadChildren: 'lazy-route/lazy.module#LazyModule',
}]
```

This "magic string" configuration was previously necessary as there was
no dynamic module loading standard on the web. This has changed with the
pending standardization of dynamic `import()` expressions, which are now
supported in the Angular CLI and in web tooling in general. `import()`
offers a more natural and robust solution to dynamic module loading. The
above example can be rewritten to use dynamic `import()`:

```
[{
  path: 'lazy',
  loadChildren: () => import('./lazy-route/lazy.module').then(mod => mod.LazyModule),
}]
```

This form of lazy loading offers significant advantages in terms of:

* type checking via TypeScript
* simplicity of generated code
* future potential to run natively in supporting browsers
  (see: [caniuse: dynamic import()](https://caniuse.com/#feat=es6-module-dynamic-import))

As a result, Angular is deprecating the `loadChildren: string` syntax in
favor of ES dynamic `import()`. An automatic migration will run during
`ng upgrade` to convert your existing Angular code to the new syntax.

PR Close angular#30073
@bvandenbon
Copy link

Do you have a preview of what lazy-loading routing files should look like in future:
I made a stackoverflow ticket for it, perhaps more convenient.

https://stackoverflow.com/questions/56272160/loadchildren-is-deprecated-in-angular8

@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 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: router cla: yes merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants