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(router): support non-NgFactory promise in loadChildren typings #29832

Conversation

Projects
None yet
5 participants
@filipesilva
Copy link
Member

commented Apr 11, 2019

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@filipesilva filipesilva requested review from angular/fw-public-api as code owners Apr 11, 2019

@googlebot googlebot added the cla: yes label Apr 11, 2019

export type LoadChildrenCallback = () => Type<any>| NgModuleFactory<any>|
Promise<NgModuleFactory<any>>| Promise<Type<any>>| Observable<Type<any>>;
export type LoadChildrenCallback = () => Type<any>| NgModuleFactory<any>| Observable<Type<any>>|
Promise<NgModuleFactory<any>| Type<any>| any>;

This comment has been minimized.

Copy link
@filipesilva

filipesilva Apr 11, 2019

Author Member

@alxhub is there a something better than Promise<any> for JIT NgModules here?

This comment has been minimized.

Copy link
@filipesilva

filipesilva Apr 13, 2019

Author Member

Examples of errors due to the missing typings can be seen on the failing CLI tests: https://circleci.com/gh/angular/angular-cli/51697 and https://circleci.com/gh/angular/angular-cli/51700.

@filipesilva filipesilva requested a review from alxhub Apr 11, 2019

@mary-poppins

This comment has been minimized.

Copy link

commented Apr 11, 2019

@filipesilva filipesilva force-pushed the filipesilva:loadchildren-nonfactory-promise branch from c96947b to a498f7e Apr 11, 2019

@mary-poppins

This comment has been minimized.

Copy link

commented Apr 11, 2019

filipesilva added a commit to filipesilva/angular-cli that referenced this pull request Apr 13, 2019

test: workaround Router typings
This workaround should be removed after angular/angular#29832 is released.

filipesilva added a commit to filipesilva/angular-cli that referenced this pull request Apr 13, 2019

test: workaround Router typings
This workaround should be removed after angular/angular#29832 is released.

filipesilva added a commit to filipesilva/angular-cli that referenced this pull request Apr 13, 2019

test: workaround Router typings
This workaround should be removed after angular/angular#29832 is released.

filipesilva added a commit to filipesilva/angular-cli that referenced this pull request Apr 13, 2019

test: workaround Router typings
This workaround should be removed after angular/angular#29832 is released.

alexeagle added a commit to angular/angular-cli that referenced this pull request Apr 13, 2019

test: workaround Router typings
This workaround should be removed after angular/angular#29832 is released.
@alxhub

alxhub approved these changes Apr 16, 2019

@@ -97,8 +97,8 @@ export type ResolveData = {
* @see `Route#loadChildren`.
* @publicApi
*/
export type LoadChildrenCallback = () => Type<any>| NgModuleFactory<any>|
Promise<NgModuleFactory<any>>| Promise<Type<any>>| Observable<Type<any>>;
export type LoadChildrenCallback = () => Type<any>| NgModuleFactory<any>| Observable<Type<any>>|

This comment has been minimized.

Copy link
@alxhub

alxhub Apr 16, 2019

Contributor

The |any in Promise<NgModuleFactory | Type | any> makes the other two redundant. They're useful for documentation purposes, I suppose.

This comment has been minimized.

Copy link
@filipesilva

filipesilva Apr 16, 2019

Author Member

Yeah that's why I didn't want to remove them. If you think it should be different, I'll change.

@kara

kara approved these changes Apr 16, 2019

@alxhub alxhub closed this in 2bfb6a0 Apr 16, 2019

@filipesilva filipesilva deleted the filipesilva:loadchildren-nonfactory-promise branch Apr 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.