Skip to content

refactor: remove unsupported Angular imports #950

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

Merged
merged 2 commits into from
Nov 25, 2021

Conversation

atscott
Copy link
Contributor

@atscott atscott commented Sep 30, 2021

Angular v13 removes the deprecated loadChildren: string syntax as well
as the supporting NgModuleFactoryLoader and SystemJsNgModuleLoader.
This commit simply removes those imports and providers. None of the
examples or tests appear to be using the loadChildren: string syntax.
In fact, the Ng2StateDeclaration.loadChildren does not even have
string as a valid type in ModuleTypeCallback.

See angular/angular@361273f

fixes #949

Angular v13 removes the deprecated `loadChildren: string` syntax as well
as the supporting `NgModuleFactoryLoader` and `SystemJsNgModuleLoader`.
This commit simply removes those imports and providers. None of the
examples or tests appear to be using the `loadChildren: string` syntax.
In fact, the `Ng2StateDeclaration.loadChildren` does not even have
`string` as a valid type in `ModuleTypeCallback`.

See angular/angular@361273f

fixes ui-router#949
@atscott atscott changed the title feat: support Angular 13 refactor: remove unsupported imports from Angular Sep 30, 2021
@atscott atscott changed the title refactor: remove unsupported imports from Angular refactor: remove unsupported Angular imports Sep 30, 2021
@kilatib
Copy link

kilatib commented Nov 8, 2021

Will you have a plan to merge that PR?

@kilatib
Copy link

kilatib commented Nov 8, 2021

I think package.json should be updated too for have the newest version there too

@mokipedia
Copy link

minimal change makes for a faster release.

would love to see this merged!

@atscott
Copy link
Contributor Author

atscott commented Nov 10, 2021

@kilatib, What @mokipedia said is the goal I was going for. I don't want to touch anything more than is absolutely necessary. These imports are leftover from old implementations and all we need to do is remove them to make everything happy.

@DOrlov77
Copy link

Hello, Angular 13 is here ;)
Do you plan to apply this fix?

@kilatib
Copy link

kilatib commented Nov 10, 2021

@atscott Do you know a way how I could hardly set the higher angular version in my package.json to avoid conflict version with that repo?

@KapilSachdev
Copy link

Hello @christopherthielen , What do you think about a new release with this change supporting Angular 13.

@DOrlov77
Copy link

Hello @antur84, may you please review the changes?

Copy link

@antur84 antur84 left a comment

Choose a reason for hiding this comment

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

Tests are still green, LGTM

@antur84
Copy link

antur84 commented Nov 23, 2021

Hello @antur84, may you please review the changes?

Sure they look fine to me, tests are green. But I'm not in a position to merge & release this, I'm a humble consumer of the lib :)

@DOrlov77
Copy link

Ahh, sorry :)
This change is show-stopper to me to upgrade to Ng13, but, probably, it is a time switch to NgRouter...

@markhus-aurelius
Copy link

A merge of this would be so appreciated. We are blocked from upgrading to v13 currently

@timofei-iatsenko
Copy link

@wawyed @christopherthielen FYI

@christopherthielen
Copy link
Member

I'll release a new version as soon as I can, thanks for the PR!

@mergify mergify bot merged commit 2dff858 into ui-router:master Nov 25, 2021
@timofei-iatsenko
Copy link

timofei-iatsenko commented Nov 26, 2021

@christopherthielen i also willing to preapare PR with version bump and explicit support of ng13 + test

The ng13 introduced new way of distributing packages and it would be nice to build this library with actual versions of ng-packagr and angular compiler.

So it's probably would be better to wait for my PR and make release of all together

@KapilSachdev
Copy link

Hello @thekip, As pointed out by @mokipedia and the change made by @atscott, the idea behind this change was "minimal change makes for a faster release."
We should not delay the release any further as this is a blocker for many applications.
I think we should make this release in the current state and the changes you are mentioning can be added in the later releases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incompatibility with Angular v13 - removal of NgModuleFactoryLoader
9 participants