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

Dependency injection not working for derived classes in ES2015 #12760

Closed
devversion opened this issue Aug 20, 2018 · 3 comments · Fixed by #14066
Closed

Dependency injection not working for derived classes in ES2015 #12760

devversion opened this issue Aug 20, 2018 · 3 comments · Fixed by #14066
Assignees
Labels
blocked This issue is blocked by some external factor, such as a prerequisite PR

Comments

@devversion
Copy link
Member

devversion commented Aug 20, 2018

NOTE: This is a tracking issue!

Consider we have a ES2015 transpiled class for the MatRowDef. The class extends the external CdkRowDef class (which comes from another node module).

material/es2015/table.ts

class MatRowDef extends CdkRowDef {}

MatRowDef.decorators = [SOME_METADATA];
// No MatRowDef.ctorParameters because the "constructor" should be inherited.

cdk/es2015/table.ts

class CdkRowDef {
    constructor(template, _differs) {
        // Does something with template & differ.
    }
}
CdkRowDef.decorators = [SOME_METADATA];
CdkRowDef.ctorParameters = () => [
    { type: TemplateRef, },
    { type: IterableDiffers, },
];

Technically this works within ES2015 applications now. The ReflectionCapabilities from @angular/core are able to properly figure out that MatRowDef is a directive which does not have an explicit constructor, but inherits a constructor from CdkRowDef.

This is being done through a brittle Regular expression that matches ES2015 classes which such traits. See this code snippet from Angular core


But now, if we take the exact same scenario from above and just consider that we run Webpack/ or use the Angular CLI, it won't work at all.

This is because Webpack transforms the ES2015 class into something like that:

Note: This is because CdkRowDef comes from another node module

class MatRowDef extends _angular_cdk_table__WEBPACK_IMPORTED_MODULE_1__["CdkRowDef"] {}

MatRowDef.decorators = [SOME_METADATA];
// No MatRowDef.ctorParameters because the "constructor" should be inherited.

Now: The regular expression (mentioned above) does no longer match the transformed class. Meaning that Angular looks for the ctorParameters on MatRowDef and comes to the conclusion that MatRowDef does not inject anything. So, TemplateRef and IterableDiffers won't be injected, and the table won't work.


A better alternative to the brittle regular expression has been proposed with: angular/tsickle#760. This technically allows the ReflectionCapabilities to drop the regex and just check for ctorParameters to be present. A PR that takes advantage of that new possibility is already pending: See: angular/angular#22642.

This would solve the given problem that is not specific to Angular Material. Everyone that uses the CLI (or explicitly Webpack) may run into a similar issue.

Related: #11580. Related #9329.

@devversion devversion self-assigned this Aug 20, 2018
devversion added a commit to devversion/material2 that referenced this issue Aug 20, 2018
Fixes that some components does not render properly when used inside of a ES2015 JIT application. This is due to an ongoing Angular issue which has been caused due to a brittle Regex check [introduced here](angular/angular#22356 (comment)) that should be replaced with a more clean approach for identifying classes that inherit metadata from another class.

* angular/angular#22642
* angular/tsickle#760

See the more detailed issue here: angular#12760

Fixes angular#9329.
jelbourn pushed a commit that referenced this issue Aug 21, 2018
Fixes that some components does not render properly when used inside of a ES2015 JIT application. This is due to an ongoing Angular issue which has been caused due to a brittle Regex check [introduced here](angular/angular#22356 (comment)) that should be replaced with a more clean approach for identifying classes that inherit metadata from another class.

* angular/angular#22642
* angular/tsickle#760

See the more detailed issue here: #12760

Fixes #9329.
jelbourn pushed a commit that referenced this issue Aug 29, 2018
Fixes that some components does not render properly when used inside of a ES2015 JIT application. This is due to an ongoing Angular issue which has been caused due to a brittle Regex check [introduced here](angular/angular#22356 (comment)) that should be replaced with a more clean approach for identifying classes that inherit metadata from another class.

* angular/angular#22642
* angular/tsickle#760

See the more detailed issue here: #12760

Fixes #9329.
@devversion
Copy link
Member Author

devversion commented Sep 6, 2018

Note: We've applied a workaround to fix this issue from our side.

  • See the according commit: 2adced1. Released in 7.0.0-beta.0.

I will keep that issue open because at some point we want to remove the workarounds and this should just remind us of the workaround still being applied.

@devversion
Copy link
Member Author

devversion commented Oct 20, 2018

Small update. Looks like the workaround those not work properly for classes with members that will be created within the constructor.

Input (TS)

export class MatTable<T> extends _CdkTable<T> {
  /** Overrides the sticky CSS class set by the `CdkTable`. */
  protected stickyCssClass = 'mat-table-sticky';
}

Result (ES2015)**

class MatTable extends _CdkTable {
    constructor() {
        super(...arguments);
        /**
         * Overrides the sticky CSS class set by the `CdkTable`.
         */
        this.stickyCssClass = 'mat-table-sticky';
    }
}

Since this no longer matches the previously mentioned regular expression for determining classes which have an inherited pass-through constructor, Angular looks for ctorParameters on MatTable.

But since the constructor is generated dynamically and just uses arguments , there is no ctorParameters metadata available.

A workaround is to manually refer to the ctorParameters from CdkTable. It's important that the value of the MatTable.ctorParameters is different to the ctorParameters from CdkTable because otherwise the parameters will be ignored.

devversion added a commit to devversion/material2 that referenced this issue Oct 20, 2018
Fixes that Angular Material cannot be used with ES2015. See angular#12760 for detailed information. We already had a workaround applied, but this improves the workaround because it only partially worked for classes where no attributes were defined.

Fixes angular#13695.
devversion added a commit to devversion/material2 that referenced this issue Oct 20, 2018
Fixes that Angular Material cannot be used with ES2015. See angular#12760 for detailed information. We already had a workaround applied, but this improves the workaround because it only partially worked for classes where no attributes were defined.

Fixes angular#13695.
mmalerba pushed a commit that referenced this issue Oct 23, 2018
Fixes that Angular Material cannot be used with ES2015. See #12760 for detailed information. We already had a workaround applied, but this improves the workaround because it only partially worked for classes where no attributes were defined.

Fixes #13695.
mmalerba pushed a commit that referenced this issue Oct 26, 2018
Fixes that Angular Material cannot be used with ES2015. See #12760 for detailed information. We already had a workaround applied, but this improves the workaround because it only partially worked for classes where no attributes were defined.

Fixes #13695.
devversion added a commit to devversion/material2 that referenced this issue Nov 9, 2018
With two commits (2adced1 and a22a9fa), we recently tried to add a workaround for an Angular bug that causes Angular Material to not work w/ ES2015. The workaround technically worked but had to be partially reverted because Google Closure compiler was no longer able to remove unused classes (due to a function with side-effects that modified the actual component metadata). Since the workaround (in the state it is right now) is not helping at all and a fix will land with Angular `v7.1.0` (angular/angular@95743e3)

Closes angular#12760
vivian-hu-zz pushed a commit that referenced this issue Nov 10, 2018
With two commits (2adced1 and a22a9fa), we recently tried to add a workaround for an Angular bug that causes Angular Material to not work w/ ES2015. The workaround technically worked but had to be partially reverted because Google Closure compiler was no longer able to remove unused classes (due to a function with side-effects that modified the actual component metadata). Since the workaround (in the state it is right now) is not helping at all and a fix will land with Angular `v7.1.0` (angular/angular@95743e3)

Closes #12760
vivian-hu-zz pushed a commit that referenced this issue Nov 12, 2018
With two commits (2adced1 and a22a9fa), we recently tried to add a workaround for an Angular bug that causes Angular Material to not work w/ ES2015. The workaround technically worked but had to be partially reverted because Google Closure compiler was no longer able to remove unused classes (due to a function with side-effects that modified the actual component metadata). Since the workaround (in the state it is right now) is not helping at all and a fix will land with Angular `v7.1.0` (angular/angular@95743e3)

Closes #12760
@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 10, 2019
@mmalerba mmalerba added the blocked This issue is blocked by some external factor, such as a prerequisite PR label Mar 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
blocked This issue is blocked by some external factor, such as a prerequisite PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants