fix(compiler): inherit param types when class has a constructor which… #29232

bolu · 2019-03-11T20:53:15Z

… takes no declared parameters and delegates up

In ReflectionCapabilities, when checking for own parameters of a type, inherit the types properly for classes that do have a constructor, but the constructor takes no declared parameters and just delegates to super(...arguments). This removes the need to declare trivial constructors in such classes to make them work properly in JIT mode. Without this, DI fails and injectables are undefined.

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

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

kara

LGTM

alxhub

Hi @bolu,

How hard do you think it would be to add a test for this fix?

alxhub

(for the test)

alxhub

Also, please merge your two commits into one single commit. git rebase -i is useful for this.

kara · 2019-03-12T18:28:32Z

presubmit

matsko · 2019-03-14T21:00:04Z

http://test/OCL:238513286:BASE:238513362:1552597194178:27e13635

… takes no declared parameters and delegates up In ReflectionCapabilities, when checking for own parameters of a type, inherit the types properly for classes that do have a constructor, but the constructor takes no declared parameters and just delegates to super(...arguments). This removes the need to declare trivial constructors in such classes to make them work properly in JIT mode. Without this, DI fails and injectables are undefined.

…r which takes no declared parameters and delegates up

googlebot · 2019-03-21T23:06:48Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

ℹ️ Googlers: Go here for more info.

googlebot · 2019-03-21T23:14:33Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

… takes no declared parameters and delegates up (angular#29232) In ReflectionCapabilities, when checking for own parameters of a type, inherit the types properly for classes that do have a constructor, but the constructor takes no declared parameters and just delegates to super(...arguments). This removes the need to declare trivial constructors in such classes to make them work properly in JIT mode. Without this, DI fails and injectables are undefined. PR Close angular#29232

petebacondarwin · 2019-05-09T20:25:43Z

Here are the tests that never appeared in this PR: #30368

angular-automatic-lock-bot · 2019-09-15T01:51:30Z

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.}

bolu requested a review from a team as a code owner March 11, 2019 20:53

googlebot added the cla: yes label Mar 11, 2019

kara added the area: core Issues related to the framework runtime label Mar 12, 2019

ngbot bot added this to the needsTriage milestone Mar 12, 2019

kara approved these changes Mar 12, 2019

View reviewed changes

kara added the action: review The PR is still awaiting reviews from at least one requested reviewer label Mar 12, 2019

kara requested a review from alxhub March 12, 2019 18:23

kara added the target: major This PR is targeted for the next major release label Mar 12, 2019

kara removed the request for review from alxhub March 12, 2019 18:25

kara added action: presubmit The PR is in need of a google3 presubmit action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Mar 12, 2019

alxhub reviewed Mar 12, 2019

View reviewed changes

alxhub requested changes Mar 12, 2019

View reviewed changes

kara added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: merge The PR is ready for merge by the caretaker labels Mar 12, 2019

kara added the action: merge The PR is ready for merge by the caretaker label Mar 12, 2019

bolu and others added 2 commits March 21, 2019 16:06

fixup! fix(compiler): inherit param types when class has a constructo…

457923f

…r which takes no declared parameters and delegates up

googlebot added cla: no and removed cla: yes labels Mar 21, 2019

mhevery added cla: yes and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews cla: no labels Mar 21, 2019

alxhub approved these changes Mar 22, 2019

View reviewed changes

mhevery closed this in 0007564 Mar 22, 2019

This was referenced May 9, 2019

reflection-capabilities: parameters not inherited in es2015 for delegate constructors. #27267

Closed

fix(core): add support for ES2015 constructor delegation #24156

Closed

CreepyGnome mentioned this pull request May 13, 2019

fix: material not working with ES2015 angular/components#13709

Merged

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 15, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(compiler): inherit param types when class has a constructor which… #29232

fix(compiler): inherit param types when class has a constructor which… #29232

bolu commented Mar 11, 2019

kara left a comment

alxhub left a comment

alxhub left a comment

alxhub left a comment

kara commented Mar 12, 2019

matsko commented Mar 14, 2019 •

edited

googlebot commented Mar 21, 2019

googlebot commented Mar 21, 2019

petebacondarwin commented May 9, 2019

angular-automatic-lock-bot bot commented Sep 15, 2019

fix(compiler): inherit param types when class has a constructor which… #29232

fix(compiler): inherit param types when class has a constructor which… #29232

Conversation

bolu commented Mar 11, 2019

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

kara left a comment

Choose a reason for hiding this comment

alxhub left a comment

Choose a reason for hiding this comment

alxhub left a comment

Choose a reason for hiding this comment

alxhub left a comment

Choose a reason for hiding this comment

kara commented Mar 12, 2019

matsko commented Mar 14, 2019 • edited

googlebot commented Mar 21, 2019

googlebot commented Mar 21, 2019

petebacondarwin commented May 9, 2019

angular-automatic-lock-bot bot commented Sep 15, 2019

matsko commented Mar 14, 2019 •

edited