Skip to content

Conversation

ts2do
Copy link

@ts2do ts2do commented May 28, 2018

The change adds better support for native ES2015 targets. The previous
behavior didn't detect parameter delegation of constructor-less classes
containing at least one inline property initialization before
compilation.

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[x] 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?

The following TypeScript code would incorrectly discard Parent's ctorParameters when compiled targeting ES2015, causing a runtime error when instantiated. See issue Dependency injection does not work with es2015 target in angular/material2.

class Parent {
  constructor(protected dep: Dep) { }
}
class Child extends Parent {
  x = 10;
}

What is the new behavior?

The above case should now be handled properly.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

The change adds better support for native ES2015 targets. The previous
behavior didn't detect parameter delegation of constructor-less classes
containing at least one inline property initialization before
compilation.
@trotyl
Copy link
Contributor

trotyl commented May 28, 2018

Relates to #22642, #24014.

Better to land #22642 first as it's the real fix.

@petebacondarwin
Copy link
Contributor

Hi @ts2do - are you interested in working more on this PR to get it merged? I see that your repository has disappeared so I guess that you may not be working with Angular any more?
In the meantime, I will take your changes and fix up the conflicts into a new PR.

@mbenna
Copy link

mbenna commented May 9, 2019

@petebacondarwin this problem may have been resolved in angular 8 at around beta 9 or thereabouts. Best to re-test with one if the 8 rc's before devoting too much time to it.

@petebacondarwin
Copy link
Contributor

Yeah, I believe this fixed it #29232

petebacondarwin added a commit to petebacondarwin/angular that referenced this pull request May 9, 2019
This commit moves the delegated constructor detection to a helper
function and also adds more test coverage.

The original code for this came from angular#24156
thanks to @ts2do.

Closes angular#24156
Closes angular#27267

// FW-1310
@alxhub alxhub closed this in b688502 May 10, 2019
BioPhoton pushed a commit to BioPhoton/angular that referenced this pull request May 21, 2019
…ngular#30368)

This commit moves the delegated constructor detection to a helper
function and also adds more test coverage.

The original code for this came from angular#24156
thanks to @ts2do.

Closes angular#24156
Closes angular#27267

// FW-1310

PR Close angular#30368
@angular-automatic-lock-bot
Copy link

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
area: core Issues related to the framework runtime cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants