[@types/lodash] Make sure readonly arrays can't be modified

[DonaldDuck313]

These methods mutate the array they take as a parameter. Since ReadonlyArray can be converted to List, the old version would allow mutating readonly arrays. With this change, these methods can only take Array parameters which doesn't accept ReadonlyArray.

Please fill in this template.

• Use a meaningful title for the pull request. Include the name of the package modified.
• Test the change in your own code. (Compile and run.)
• Add or edit tests to reflect the change.
• Follow the advice from the readme.
• Avoid common mistakes.
• Run pnpm test <package to test>.

Select one of these and delete the others:

If adding a new definition: Not applicable

If changing an existing definition:

• Provide a URL to documentation or source code which provides context for the suggested changes: https://lodash.com/docs/4.17.15
• If this PR brings the type definitions up to date with a new version of the JS library, update the version number in the package.json. (Not applicable, this was a bug in DefinitelyTyped)

If removing a declaration: Not applicable

[typescript-bot]

@DonaldDuck313 Thank you for submitting this PR!

This is a live comment that I will keep updated.

1 package in this PR

• lodash — on npm, on unpkg

Code Reviews

Because this is a widely-used package, a DT maintainer will need to review it before it can be merged.

You can test the changes of this PR in the Playground.

Status

• ✅ No merge conflicts
• ❌ Continuous integration tests have failed
• 🕐 Most recent commit is approved by a DT maintainer

Once every item on this list is checked, I'll ask you for permission to merge and publish the changes.

---

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "now": "-",
  "pr_number": 72536,
  "author": "DonaldDuck313",
  "headCommitOid": "faba671aa880ec7f9da19c4c71a7bb0bfdad7ad5",
  "mergeBaseOid": "5d29b9be383902b0399f26072b4590ac61ca72c5",
  "lastPushDate": "2025-04-17T16:59:14.000Z",
  "lastActivityDate": "2025-05-26T11:29:41.000Z",
  "hasMergeConflict": false,
  "isFirstContribution": false,
  "tooManyFiles": false,
  "hugeChange": false,
  "popularityLevel": "Critical",
  "pkgInfo": [
    {
      "name": "lodash",
      "kind": "edit",
      "files": [
        {
          "path": "types/lodash/common/array.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/lodash/lodash-tests.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "bczengel",
        "chrootsu",
        "aj-r",
        "e-cloud",
        "jtmthf",
        "DomiR",
        "WilliamChelman"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Critical"
    }
  ],
  "reviews": [
    {
      "type": "stale",
      "reviewer": "aj-r",
      "date": "2025-05-10T11:03:35.000Z",
      "abbrOid": "c297f3f"
    }
  ],
  "mainBotCommentID": 2813559509,
  "ciResult": "fail",
  "ciUrl": "https://github.com/DefinitelyTyped/DefinitelyTyped/commit/faba671aa880ec7f9da19c4c71a7bb0bfdad7ad5/checks?check_suite_id=39074608535"
}

[typescript-bot]

🔔 @bczengel @chrootsu @aj-r @e-cloud @jtmthf @DomiR @WilliamChelman — please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

[typescript-bot]

@DonaldDuck313 The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

Note: builds that are failing do not end up on the list of PRs for the DT maintainers to review.

[aj-r]

Thanks! Please also update fp.d.ts (in the root lodash folder). You can do it by hand, the fp-generator doesn't work anymore.

[aj-r]

This one is tricky because while we shouldn't support readonly arrays, we should support mutable array-like objects that can be indexed by number. Maybe we need a new MutableList interface, e.g.

interface MutableList<T> {
    readonly length: number;
    [n: number]: T;
}

[DonaldDuck313]

That suggestion doesn't work, and it still doesn't work if I use length instead of readonly length. After some googling I found that there isn't any way to do this, so I guess it's up to you to decide whether you think it's more important to support array-like objects or to not support readonly arrays. I personally think it's more important to not support readonly arrays because I've never actually seen a use case for mutable array-like objects.

[aj-r]

I'd rather err on the side of not breaking existing code, i.e. keep supporting array-like objects. While rare, somebody is probably using them (and there are some native array-like types such as NodeList and TypedArray that are very likely in use).

So let's either add this overload back, or use the MutableList approach in hopes that Microsoft will eventually fix microsoft/TypeScript#13347, or at least allow people to use linters to catch the error.

Alternatively, you could try adding an extra overload above this one to "trap" readonly arrays and return never, e.g.

fill(array: readonly any[] | null | undefined, value: any): never;

[typescript-bot]

@DonaldDuck313 One or more reviewers has requested changes. Please address their comments. I'll be back once they sign off or you've pushed new commits. Thank you!

[typescript-bot]

@DonaldDuck313 The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

Note: builds that are failing do not end up on the list of PRs for the DT maintainers to review.

[DonaldDuck313]

Thanks! Please also update fp.d.ts (in the root lodash folder). You can do it by hand, the fp-generator doesn't work anymore.

I tried that but it seems to have broken the tests. I don't quite understand what fp.d.ts does, any idea what's wrong?

[aj-r]

I tried that but it seems to have broken the tests. I don't quite understand what fp.d.ts does, any idea what's wrong?

FP is an alternative "functional programming" interface for lodash: https://github.com/lodash/lodash/wiki/fp-guide. It tends to curry and reverse the order of arguments to support a more functional-style paradigm.

However, I just noticed that fp.pull seems to be creating a copy of the input instead of mutating it (since functional programming generally doesn't allow mutation). Sorry for the confusion, you can probably revert the FP change.

[aj-r]

I'd rather err on the side of not breaking existing code, i.e. keep supporting array-like objects. While rare, somebody is probably using them (and there are some native array-like types such as NodeList and TypedArray that are very likely in use).

So let's either add this overload back, or use the MutableList approach in hopes that Microsoft will eventually fix microsoft/TypeScript#13347, or at least allow people to use linters to catch the error.

Alternatively, you could try adding an extra overload above this one to "trap" readonly arrays and return never, e.g.

fill(array: readonly any[] | null | undefined, value: any): never;

[typescript-bot]

@DonaldDuck313 One or more reviewers has requested changes. Please address their comments. I'll be back once they sign off or you've pushed new commits. Thank you!

[typescript-bot]

Re-ping @bczengel, @chrootsu, @aj-r, @e-cloud, @jtmthf, @DomiR, @WilliamChelman:

This PR has been out for over a week, yet I haven't seen any reviews.

Could someone please give it some attention? Thanks!

[typescript-bot]

It has been more than two weeks and this PR still has no reviews.

I'll bump it to the DT maintainer queue. Thank you for your patience, @DonaldDuck313.

(Ping @bczengel, @chrootsu, @aj-r, @e-cloud, @jtmthf, @DomiR, @WilliamChelman.)

[aj-r]

Sorry for the late review. Nice solution! Can you add tests for this, e.g. use @ts-expect-error to verify that readonly arrays are rejected?

[typescript-bot]

@DonaldDuck313 One or more reviewers has requested changes. Please address their comments. I'll be back once they sign off or you've pushed new commits. Thank you!

[typescript-bot]

@DonaldDuck313 The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

Note: builds that are failing do not end up on the list of PRs for the DT maintainers to review.

[typescript-bot]

@DonaldDuck313 The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

Note: builds that are failing do not end up on the list of PRs for the DT maintainers to review.

[DonaldDuck313]

@aj-r I fixed the tests so that they should work, but they're giving strange errors that I don't understand. At line 907 (a blank line) it says "Unused @ts-expect-error". What does "Unused @ts-expect-error" even mean? And why does it give that error at a blank line?

[aj-r]

I think it means that the line doesn't contain any error. I'm not sure why it would trigger on a blank line, maybe it's giving you the wrong line number?