Skip to content

fix(core): handle undefined meta in injectArgs - #31333

Closed
alan-agius4 wants to merge 1 commit into
angular:masterfrom
alan-agius4:reflector
Closed

alan-agius4 wants to merge 1 commit into
angular:masterfrom
alan-agius4:reflector

Conversation

@alan-agius4

Copy link
Copy Markdown
Contributor

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 a team June 28, 2019 10:57
@alan-agius4 alan-agius4 added target: patch This PR is targeted for the next patch release and removed cla: yes labels Jun 28, 2019
@googlebot

Copy link
Copy Markdown

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

@alan-agius4 alan-agius4 added PR action: time-zone action: review The PR is still awaiting reviews from at least one requested reviewer area: core Issues related to the framework runtime labels Jun 28, 2019
@ngbot ngbot Bot added this to the needsTriage milestone Jun 28, 2019
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 removed the action: review The PR is still awaiting reviews from at least one requested reviewer label Jun 28, 2019
@alxhub

alxhub commented Jun 28, 2019

Copy link
Copy Markdown
Member

Presubmit

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

@alan-agius4 alan-agius4 added action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note labels Jun 29, 2019
@alan-agius4

Copy link
Copy Markdown
Contributor Author

Caretaker: codefresh seems to be not reporting the status.

@alxhub alxhub closed this in f83dfd6 Jul 1, 2019
alxhub pushed a commit that referenced this pull request Jul 1, 2019
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 reflector branch July 1, 2019 19:04
@angular-automatic-lock-bot

Copy link
Copy Markdown

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: core Issues related to the framework runtime 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: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot read property 'ngMetadataName' of undefined with @angular-devkit/build-angular 0.800.4

3 participants