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

feat(core): support usage of non-experimental decorators with TypeScript 5.0 #49492

Closed
wants to merge 1 commit into from

Conversation

clydin
Copy link
Member

@clydin clydin commented Mar 20, 2023

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?

Previously, attempting to turn off the experimentalDecorators TypeScript configuration option within an Angular project would result in build time errors. These errors were due to an exposed Decorator signature from @angular/core that TypeScript thought was incompatible with standard decorators. However, Angular's class decorators (Component, Directive, Pipe, Injectable, NgModule) are actually already compatible with standard decorators. The export types for the decorators only needed to be updated to reflect that compatibility. With the updated exported types, applications will now successfully compile and execute in AOT mode with one important dependency injection caveat explained in the note below.
For JIT mode applications that are built with the Angular CLI, @ngtools/webpack, or use tsickle, there were also no additional changes required with the same dependency injection caveat. These tools automatically convert property decorators (now called field decorators) at build time to store Angular property metadata directly on the relevant class. Building with these tools is the overwhelmingly common method of building an application. Any applications that do not use one of these tools will not function at runtime in JIT mode if using standard decorators. The behavior and code for when experimental decorators is enabled has been left intact.

NOTE: Angular constructor dependency injection that requires parameter decorators is not supported. The standard decorator specification does not support parameter decorators. The inject function must be used for all cases that previously required a parameter decorator. This includes such decorators as Inject, Optional, Self, SkipSelf, Host, and Attribute. Constructor dependency injection that relies only on the supplied parameter type will continue to function as expected if using AOT; as well as in JIT mode if using the Angular CLI, @ngtools/webpack directly, or tsickle.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Documentation for the inject function can be found at: https://angular.io/api/core/inject
The decorator specification proposal can be found at: https://github.com/tc39/proposal-decorators

An initial CLI E2E test that uses these changes is available here: angular/angular-cli#24886

@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label Mar 20, 2023
@clydin clydin added the target: major This PR is targeted for the next major release label Mar 20, 2023
@pkozlowski-opensource pkozlowski-opensource added the area: core Issues related to the framework runtime label Mar 20, 2023
@ngbot ngbot bot added this to the Backlog milestone Mar 20, 2023
@clydin clydin force-pushed the standard-decorators branch 2 times, most recently from 13cdd66 to e222e90 Compare March 20, 2023 14:19
@clydin clydin changed the title feat(@angular/core): support usage of non-experimental decorators with TypeScript 5.0 feat(core): support usage of non-experimental decorators with TypeScript 5.0 Mar 20, 2023
@clydin clydin marked this pull request as ready for review March 21, 2023 14:09
@pullapprove pullapprove bot requested a review from dylhunn March 21, 2023 14:09
@clydin clydin force-pushed the standard-decorators branch 3 times, most recently from 07a6bfc to c5e09b1 Compare March 22, 2023 14:27
@clydin clydin added the action: review The PR is still awaiting reviews from at least one requested reviewer label Mar 24, 2023
@clydin clydin force-pushed the standard-decorators branch 2 times, most recently from 597c8ef to cb3844c Compare April 1, 2023 13:54
Copy link
Contributor

@dylhunn dylhunn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewed-for: fw-core

Copy link
Contributor

@jessicajaniuk jessicajaniuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewed-for: public-api

@pullapprove pullapprove bot requested a review from alxhub April 3, 2023 20:45
Copy link
Member

@alxhub alxhub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should discuss the level of support commitment we're making in this PR for standard decorators.

Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we have some sort of test for this?

@clydin
Copy link
Member Author

clydin commented Apr 4, 2023

There is a linked CLI E2E test that demonstrates the functionality. If there is a more appropriate place for some tests, i can add them as well.

packages/core/src/errors.ts Outdated Show resolved Hide resolved
packages/core/src/util/decorators.ts Outdated Show resolved Hide resolved
@AndrewKushnir
Copy link
Contributor

AndrewKushnir commented Apr 4, 2023

@clydin it looks like we the commit message might need an update as well, since the implementation has changed.

@clydin clydin force-pushed the standard-decorators branch 2 times, most recently from 6d5d780 to 4069209 Compare April 5, 2023 00:14
…ipt 5.0

Previously, attempting to turn off the `experimentalDecorators` TypeScript configuration
option within an Angular project would result in build time errors. These errors were due
to an exposed Decorator signature from `@angular/core` that TypeScript thought was incompatible
with standard decorators. However, Angular's class decorators (`Component`, `Directive`, `Pipe`,
`Injectable`, `NgModule`) are actually already compatible with standard decorators. The export types for
the decorators only needed to be updated to reflect that compatibility. With the updated exported types,
applications will now successfully compile and execute in AOT mode with one important dependency injection
caveat explained in the note below.
For JIT mode applications that are built with the Angular CLI, `@ngtools/webpack`, or use `tsickle`,
there were also no additional changes required. These tools automatically convert property decorators
(now called field decorators) at build time to store Angular property metadata directly on the relevant
class. Building with these tools is the overwhelmingly common method of building an application. Any
applications that do not use one of these tools will not function at runtime in JIT mode if using standard
decorators. The behavior and code for when experimental decorators is enabled has been left intact.

NOTE: Angular constructor dependency injection that requires parameter decorators is not supported.
The standard decorator specification does not support parameter decorators. The `inject` function must be
used for all cases that previously required a parameter decorator. This includes such decorators as `Inject`,
`Optional`, `Self`, `SkipSelf`, `Host`, and `Attribute`. Constructor dependency injection that relies only
on the supplied parameter type will continue to function as expected if using AOT; as well as in JIT mode
if using the Angular CLI, `@ngtools/webpack` directly, or `tsickle`.

Documentation for the `inject` function can be found at: https://angular.io/api/core/inject
The decorator specification proposal can be found at: https://github.com/tc39/proposal-decorators
@clydin
Copy link
Member Author

clydin commented Apr 5, 2023

PR has been updated to throw instead and now contains much less code.

@dylhunn
Copy link
Contributor

dylhunn commented Apr 5, 2023

@clydin Is this still targeted for v16? Just checking in, since freeze is today. Happy to push on it or contributed in any way you need.

@pullapprove pullapprove bot requested a review from alxhub April 5, 2023 16:18
@dylhunn dylhunn added 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 Apr 5, 2023
@dylhunn
Copy link
Contributor

dylhunn commented Apr 5, 2023

This PR was merged into the repository by commit aad05eb.

@dylhunn dylhunn closed this in aad05eb Apr 5, 2023
@clydin clydin deleted the standard-decorators branch April 26, 2023 16:19
@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 May 27, 2023
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 detected: feature PR contains a feature commit target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants