- 
                Notifications
    You must be signed in to change notification settings 
- Fork 13.1k
          Fixes for decorators property deprecations
          #50343
        
          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
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
decoratorsproperty onNode:First, the deprecated
decoratorsproperty used a type that made it difficult to find runtime breaks since use ofdecoratorswould silently succeed rather than fail to compile. To address this, I've changed the typeofdecoratorsfromNodeArray<Decorator> | undefinedtoundefinedto make these breaks more obvious. As a result of this change, I renamed thedecoratorsproperty to_decoratorsinternally 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
createConstructorDeclarationandupdateConstructorDeclarationfunctions ofNodeFactoryincorrectly rejected a valid value forbodywhen using the deprecated overloads due to an incorrect negation of a test.Fixes #50259