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

Angular Non AOT compiler doesn't emit class decorators in specific cases #39574

Closed
ratoaq2 opened this issue Nov 5, 2020 · 6 comments
Closed
Labels
area: compiler Issues related to `ngc`, Angular's template compiler P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent state: has PR
Milestone

Comments

@ratoaq2
Copy link

ratoaq2 commented Nov 5, 2020

🐞 bug report

Affected Package

Angular Non AOT compiler (Jit Mode)

Is this a regression?

Yes, the very same scenario works on Angular 8.x

Description

Given that AOT is disabled ("aot": false in angular.json)
Given a class that is responsible of configuring "class decorators", if I decorate a regular typescript class with a decorator generated by that class, the compiled javascript doesn't emit the class decorator properly.

class DecoratorBuilder {
  customClassDecorator<T extends { new (...args: any[]): {} }>(constructor: T) {
    console.log('Class Decorated');
    return class extends constructor {
      myName = 'Decorated Foobar';
    };
  }
}

export function ClassDecorators() {
  return new DecoratorBuilder();
}

@ClassDecorators().customClassDecorator
class MyNonAngularClass {
  myName = 'foobar';
}

🔬 Minimal Reproduction

https://github.com/ratoaq2/angular-non-aot-class-decorator

Just start the app:
npm install
npm start

access http://localhost:4200
You should see: Hello Decorated Foobar!
But you see: Hello foobar!

If you enable "aot": true, it works correctly. But that's not an option because I also face this issue in my spec tests where I can't enable aot.

🔥 Exception or Error

No exception or error

🌍 Your Environment

Angular Version:


Angular CLI: 10.0.8
Node: 12.18.4
OS: win32 x64

Angular: 10.0.14
... animations, common, compiler, compiler-cli, core, forms
... platform-browser, platform-browser-dynamic, router
Ivy Workspace: No

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.1000.8
@angular-devkit/build-angular     0.1000.8
@angular-devkit/build-optimizer   0.1000.8
@angular-devkit/build-webpack     0.1000.8
@angular-devkit/core              10.0.8
@angular-devkit/schematics        10.0.8
@angular/cli                      10.0.8
@ngtools/webpack                  10.0.8
@schematics/angular               10.0.8
@schematics/update                0.1000.8
rxjs                              6.5.5
typescript                        3.9.7
webpack                           4.43.0

Anything else relevant?
It doesn't seem to be a typescript issue

@petebacondarwin
Copy link
Member

I think that this is more related to the CLI processing than the framework compilation. But @alan-agius4 is looking into it.
Transferring to the CLI repo...

@petebacondarwin petebacondarwin transferred this issue from angular/angular Nov 5, 2020
@petebacondarwin
Copy link
Member

Actually it is caused by a transform (constructorParametersDownlevelTransform) that is held in the FW repository. So transferring back!

@petebacondarwin petebacondarwin transferred this issue from angular/angular-cli Nov 5, 2020
@petebacondarwin petebacondarwin added area: compiler Issues related to `ngc`, Angular's template compiler P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent labels Nov 5, 2020
@ngbot ngbot bot added this to the Backlog milestone Nov 5, 2020
petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Nov 5, 2020
There is a compiler transform that downlevels Angular class decorators
to static properties so that metadata is available for JIT compilation.
The transform was supposed to ignore non-Angular decorators but it was
actually completely dropping decorators that did not conform to a very
specific syntactic shape (i.e. the decorator was a simple identifier, or
a namespaced identifier).

This commit ensures that all non-Angular decorators are kepts as-is
even if they are built using a syntax that the Angular compiler does not
understand.

Fixes angular#39574
petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Nov 5, 2020
There is a compiler transform that downlevels Angular class decorators
to static properties so that metadata is available for JIT compilation.
The transform was supposed to ignore non-Angular decorators but it was
actually completely dropping decorators that did not conform to a very
specific syntactic shape (i.e. the decorator was a simple identifier, or
a namespaced identifier).

This commit ensures that all non-Angular decorators are kepts as-is
even if they are built using a syntax that the Angular compiler does not
understand.

Fixes angular#39574
@ratoaq2
Copy link
Author

ratoaq2 commented Nov 5, 2020

Thanks for the quick response.
Is this kind of issue going to be fixed only on latest version 10.2.x? Or can I expect it to also be fixed on 10.0.x?

@ahnpnl
Copy link
Contributor

ahnpnl commented Nov 5, 2020

Hi, will this be fixed for v9 as well ? Downlevel transformer in v9 locates in CLI repo but I don't see any PRs to fix this issue there for v9.

@petebacondarwin
Copy link
Member

The PR for this issue will land on master, RC and patch branches. The patch branch is currently 10.2.x.
It will not be backported to 10.0.x nor 9.x.

mhevery pushed a commit that referenced this issue Nov 6, 2020
…ng (#39577)

There is a compiler transform that downlevels Angular class decorators
to static properties so that metadata is available for JIT compilation.
The transform was supposed to ignore non-Angular decorators but it was
actually completely dropping decorators that did not conform to a very
specific syntactic shape (i.e. the decorator was a simple identifier, or
a namespaced identifier).

This commit ensures that all non-Angular decorators are kepts as-is
even if they are built using a syntax that the Angular compiler does not
understand.

Fixes #39574

PR Close #39577
@mhevery mhevery closed this as completed in b9b9178 Nov 6, 2020
mhevery pushed a commit that referenced this issue Nov 6, 2020
…ng (#39577)

There is a compiler transform that downlevels Angular class decorators
to static properties so that metadata is available for JIT compilation.
The transform was supposed to ignore non-Angular decorators but it was
actually completely dropping decorators that did not conform to a very
specific syntactic shape (i.e. the decorator was a simple identifier, or
a namespaced identifier).

This commit ensures that all non-Angular decorators are kepts as-is
even if they are built using a syntax that the Angular compiler does not
understand.

Fixes #39574

PR Close #39577
@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 Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: compiler Issues related to `ngc`, Angular's template compiler P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent state: has PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants