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(core): handle `undefined` meta in `injectArgs` #31333

Closed
wants to merge 1 commit into from

Conversation

@alan-agius4
Copy link
Contributor

commented Jun 28, 2019

In the recent versions of the CLI we introduced a ctor downlevel for VE JIT build based on the one found in tsickle, to fix the TDZ issue of forwardRef.

However this caused a regression as the injector is not handling that a position paramType can be undefined. Which is bubbled down to

export function injectArgs(types: (Type<any>| InjectionToken<any>| any[])[]): any[] {
and will crash
const meta = arg[j];
if (meta instanceof Optional || meta.ngMetadataName === 'Optional' || meta === Optional) {
flags |= InjectFlags.Optional;
} else if (
meta instanceof SkipSelf || meta.ngMetadataName === 'SkipSelf' || meta === SkipSelf) {
flags |= InjectFlags.SkipSelf;
} else if (meta instanceof Self || meta.ngMetadataName === 'Self' || meta === Self) {
flags |= InjectFlags.Self;
} else if (meta instanceof Inject || meta === Inject) {
type = meta.token;
} else {
type = meta;
}

Fixes angular/angular-cli#14888

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

@alan-agius4 alan-agius4 requested a review from angular/fw-core as a code owner Jun 28, 2019

@googlebot googlebot added the cla: yes label Jun 28, 2019

@googlebot googlebot added the cla: yes label Jun 28, 2019

@googlebot

This comment has been minimized.

Copy link

commented Jun 28, 2019

☹️ Sorry, but only Googlers may change the label cla: yes.

fix(core): handle `undefined` meta in `injectArgs`
In the recent versions of the CLI we introduced a ctor downleveler tranformer for VE JIT builds based on the one found in tsickle, to fix the TDZ issue of `forwardRef`.

However this caused a regression as the injector is not handling that a position `paramType` can be undefined. Which is bubbled down to https://github.com/angular/angular/blob/c6b29f4c6d23b1510db3434cb030203d5bdea119/packages/core/src/di/injector_compatibility.ts#L162 and will crash https://github.com/angular/angular/blob/c6b29f4c6d23b1510db3434cb030203d5bdea119/packages/core/src/di/injector_compatibility.ts#L174-L186

Fixes angular/angular-cli#14888

@alan-agius4 alan-agius4 force-pushed the alan-agius4:reflector branch from 08bbe99 to 34b97c5 Jun 28, 2019

@alxhub

alxhub approved these changes Jun 28, 2019

@alxhub

This comment has been minimized.

Copy link
Contributor

commented Jun 28, 2019

Presubmit

No Ivy presubmit needed - this only catches cases that would've crashed anyway.

@alan-agius4

This comment has been minimized.

Copy link
Contributor Author

commented Jun 29, 2019

Caretaker: codefresh seems to be not reporting the status.

@alxhub alxhub closed this in f83dfd6 Jul 1, 2019

alxhub added a commit that referenced this pull request Jul 1, 2019

fix(core): handle `undefined` meta in `injectArgs` (#31333)
In the recent versions of the CLI we introduced a ctor downleveler tranformer for VE JIT builds based on the one found in tsickle, to fix the TDZ issue of `forwardRef`.

However this caused a regression as the injector is not handling that a position `paramType` can be undefined. Which is bubbled down to https://github.com/angular/angular/blob/c6b29f4c6d23b1510db3434cb030203d5bdea119/packages/core/src/di/injector_compatibility.ts#L162 and will crash https://github.com/angular/angular/blob/c6b29f4c6d23b1510db3434cb030203d5bdea119/packages/core/src/di/injector_compatibility.ts#L174-L186

Fixes angular/angular-cli#14888

PR Close #31333

@alan-agius4 alan-agius4 deleted the alan-agius4:reflector branch Jul 1, 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.