Skip to content

Conversation

@ericbf
Copy link
Contributor

@ericbf ericbf commented Mar 18, 2023

Only the first parameter of copyWithin is required, contrary to the type in the typedef. This commit fixes that type to allow correct usage of the copyWithin function. See the usage of copyWithin.

Fixes #53334

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Mar 18, 2023
Only the first parameter of `copyWithin` is required, contrary to the type in the typedef. This commit fixes that type to allow correct usage of the `copyWithin`
function. See [the usage of copy within](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/copyWithin).
@ericbf ericbf force-pushed the fix-copyWithin-type branch from 35bdb24 to f581696 Compare March 18, 2023 16:27
@jakebailey jakebailey marked this pull request as draft March 22, 2023 16:52
@typescript-bot typescript-bot removed the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Mar 22, 2023
@jakebailey jakebailey marked this pull request as ready for review March 22, 2023 16:52
@typescript-bot typescript-bot added the For Backlog Bug PRs that fix a backlog bug label Mar 22, 2023
@jakebailey jakebailey requested a review from sandersn March 22, 2023 16:52
@jakebailey
Copy link
Member

The type change looks fine to me (making it optional), although I'm not sure about all of that extra JSDoc outlining the edge cases.

Copy link
Member

@RyanCavanaugh RyanCavanaugh left a comment

Choose a reason for hiding this comment

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

The linked bug is to change the signature, not the JS Doc, so please just change the signature. Please file a separate issue if you want to change the doc comments too.

@ericbf
Copy link
Contributor Author

ericbf commented Mar 22, 2023

@RyanCavanaugh

The linked bug is to change the signature, not the JS Doc, so please just change the signature. Please file a separate issue if you want to change the doc comments too.

You got it. No change at all? Or should it at least mention start's being omitted? I can update again if it’s more desirable to have no change at all in the JS Doc (despite the signature changing). But it makes sense that the JS Doc should at least mention the updated signature.

@ericbf
Copy link
Contributor Author

ericbf commented Mar 22, 2023

@jakebailey

The type change looks fine to me (making it optional), although I'm not sure about all of that extra JSDoc outlining the edge cases.

I got a little ambitious 😅. Apologies!

@RyanCavanaugh RyanCavanaugh merged commit a82b927 into microsoft:main Mar 22, 2023
@RyanCavanaugh
Copy link
Member

Thanks!

@ericbf ericbf deleted the fix-copyWithin-type branch March 22, 2023 17:56
@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 Backlog Bug PRs that fix a backlog bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Array.copyWithin() signature is incorrect

4 participants