-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
Fixes for decorators
property deprecations
#50343
Conversation
*/ | ||
readonly decorators: never; | ||
/** | ||
* @deprecated `modifiers` has been removed from `Node` and moved to the specific `Node` subtypes that support them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are modifiers
actually removed? or just more specific in their type on sub-nodes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's intended that modifiers
is not a property of every Node
, but rather is a property of specific Node
sub-types. i.e., you shouldn't just blindly read from node.modifiers
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good although I got confused by the wording of the last sentence of decorators
' jsdoc.
146f69f
to
a2588b8
Compare
@typescript-bot cherry-pick this to release-4.8 |
Heya @DanielRosenwasser, I've started to run the task to cherry-pick this into |
Hey @DanielRosenwasser, I've opened #50374 for you. |
@DanielRosenwasser, we may need to re-create or update the cherry-pick PR. |
@typescript-bot cherry-pick this to release-4.8 |
Heya @DanielRosenwasser, I've started to run the task to cherry-pick this into |
Hey @DanielRosenwasser, I couldn't open a PR with the cherry-pick. (You can check the log here). You may need to squash and pick this PR into release-4.8 manually. |
Component commits: da3e304 Change type of deprecated 'decorators' property a63f47c fix 'Invalid Arguments' error for create/update constructor in factory a2588b8 Update deprecation comments aebe225 Make 'decorators' optional and 'undefined' 46c30f3 Rename '_decorators' to 'illegalDecorators' 6404716 Update baselines
Component commits: da3e304 Change type of deprecated 'decorators' property a63f47c fix 'Invalid Arguments' error for create/update constructor in factory a2588b8 Update deprecation comments aebe225 Make 'decorators' optional and 'undefined' 46c30f3 Rename '_decorators' to 'illegalDecorators' 6404716 Update baselines Co-authored-by: Ron Buckton <rbuckton@microsoft.com>
This fixes two issues that occur in TS 4.8 related to the deprecation of the
decorators
property onNode
:First, the deprecated
decorators
property used a type that made it difficult to find runtime breaks since use ofdecorators
would silently succeed rather than fail to compile. To address this, I've changed the typeofdecorators
fromNodeArray<Decorator> | undefined
toundefined
to make these breaks more obvious. As a result of this change, I renamed thedecorators
property to_decorators
internally for cases where we track invalid grammar nodes (such as decorators parsed on a constructor, etc.), otherwise I would have run into a conflict with the definition of the types.Second, the
createConstructorDeclaration
andupdateConstructorDeclaration
functions ofNodeFactory
incorrectly rejected a valid value forbody
when using the deprecated overloads due to an incorrect negation of a test.Fixes #50259