Skip to content

Change ParamaterDecorator to allow an undefined propertyKey#53365

Merged
rbuckton merged 1 commit intomicrosoft:mainfrom
dhritzkiv:patch-2
Mar 20, 2023
Merged

Change ParamaterDecorator to allow an undefined propertyKey#53365
rbuckton merged 1 commit intomicrosoft:mainfrom
dhritzkiv:patch-2

Conversation

@dhritzkiv
Copy link
Copy Markdown
Contributor

@dhritzkiv dhritzkiv commented Mar 20, 2023

propertyKey argument can be undefined for constructor parameter decorators.

I believe changes between 4.9 and 5.0 tightened up decorator type checking in some areas (#52435), while a newly introduced declaration file for legacy decorators doesn't reflect this change.

After upgrading to 5.0, I noticed that in my code, a dependency's usage of ParameterDecorator resulted in a type checking warning:

Unable to resolve signature of parameter decorator when called as an expression.
  Argument of type 'undefined' is not assignable to parameter of type 'string | symbol'

Changing the propertyKey in decorators.legacy.d.ts to allow undefined fixed the type-checking error.

I did not touch the other Decorator types as I don't know if the propertyKey issue affects them in the same way.

Fixes #53312

`propertyKey` argument can be undefined for parameter decorators
@typescript-bot typescript-bot added the For Milestone Bug PRs that fix a bug with a specific milestone label Mar 20, 2023
@jakebailey jakebailey closed this Mar 20, 2023
@jakebailey jakebailey reopened this Mar 20, 2023
@jakebailey
Copy link
Copy Markdown
Member

(ignore the close/open; just fixing CI after I broke/unbroke main 🙃)

@dhritzkiv
Copy link
Copy Markdown
Contributor Author

OK! I was worried I did something wrong to have the tests break like that. 😌

@dhritzkiv dhritzkiv marked this pull request as ready for review March 20, 2023 04:59
@rbuckton rbuckton merged commit 41474f9 into microsoft:main Mar 20, 2023
@jakebailey
Copy link
Copy Markdown
Member

jakebailey commented Mar 20, 2023

This needs a backport to 5.0, right? @DanielRosenwasser

@rbuckton
Copy link
Copy Markdown
Contributor

@DanielRosenwasser: we may want to include this if we do any follow-up 5.0.x patches. It is a potentially breaking change, but is in line with the correctness change we made for parameter decorators that we mention in the 5.0 release notes.

@jakebailey
Copy link
Copy Markdown
Member

@typescript-bot cherry-pick this to release-5.0

@typescript-bot
Copy link
Copy Markdown
Collaborator

typescript-bot commented Mar 20, 2023

Heya @jakebailey, I've started to run the task to cherry-pick this into release-5.0 on this PR at 2c400b7. You can monitor the build here.

@typescript-bot
Copy link
Copy Markdown
Collaborator

Hey @jakebailey, I've opened #53392 for you.

typescript-bot pushed a commit to typescript-bot/TypeScript that referenced this pull request Mar 20, 2023
Component commits:
2c400b7 Update decorators.legacy.d.ts
`propertyKey` argument can be undefined for parameter decorators
@dhritzkiv dhritzkiv deleted the patch-2 branch March 20, 2023 19:14
@DanielRosenwasser
Copy link
Copy Markdown
Member

Technically any library that exports a `ParameterDecorator is potentially "wrong" today; but on the other hand, any existing code is probably sufficiently working anyway. So I think a cherry-pick is reasonable because it should unblock consumers, while helping library authors adapt.

DanielRosenwasser pushed a commit that referenced this pull request Mar 27, 2023
…e-5.0 (#53392)

Co-authored-by: Daniel Hritzkiv <daniel.hritzkiv@gmail.com>
drivron pushed a commit to scenari/typescript that referenced this pull request Sep 14, 2023
…to release-5.0 (microsoft#53392)

Co-authored-by: Daniel Hritzkiv <daniel.hritzkiv@gmail.com>
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

For Milestone Bug PRs that fix a bug with a specific milestone

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Legacy/Experimental parameter decorators fails with TS1239 when StrictNullChecks is true in Typescript 5.0.2

5 participants