Skip to content

[@types/underscore] Support readonly tuples in object calls#46650

Merged
typescript-bot merged 1 commit intoDefinitelyTyped:masterfrom
reubenrybnik:underscore-object-support-const-tuple-assertion
Aug 11, 2020
Merged

[@types/underscore] Support readonly tuples in object calls#46650
typescript-bot merged 1 commit intoDefinitelyTyped:masterfrom
reubenrybnik:underscore-object-support-const-tuple-assertion

Conversation

@reubenrybnik
Copy link
Copy Markdown
Contributor

@reubenrybnik reubenrybnik commented Aug 10, 2020 •
edited
Loading

When installing the 1.10.20 type definitions, I ran across an issue with object that I thought it would be nice to fix. By default TS typically infers the type of an array initialization as an array rather than a tuple even if it contains multiple types of objects, but TS3.4 and above supports an as const shorthand assertion that will turn any array-initializing declaration into a tuple without needing to manually specify all the types in the tuple (see below). However, the resulting tuple ends up being readonly, and type inference in PairValue doesn't currently work with readonly tuples. This fixes that issue while still maintaining support for non-readonly tuples.

// result is Dictionary<string | number> - the result of map is (string | number)[],
// so type inference can't determine what the value type is definitely going to be
_.chain([{ id: 1, name: 'a' }, { id: 2, name: 'b' }])
    .map(o => [o.name, o.id])
    .object()
    .value();

// result is Dictionary<number> - this works but it's annoying to need to specify
// the types in the tuple
_.chain([{ id: 1, name: 'a' }, { id: 2, name: 'b' }])
    .map(o => [o.name, o.id] as [string, number])
    .object()
    .value();

// result is Dictionary<string | number> - this should work but the result of map is
// `readonly [string, number]`, which does not extend `[string, number]`
_.chain([{ id: 1, name: 'a' }, { id: 2, name: 'b' }])
    .map(o => [o.name, o.id] as const)
    .object()
    .value();

@reubenrybnik
Making it so the use of the shorthand "as const" to turn an array int…
3653a84 
…o a readonly tuple works with object type definitions.
@typescript-bot typescript-bot added Popular package This PR affects a popular package (as counted by NPM download counts). Author is Owner The author of this PR is a listed owner of the package. labels Aug 10, 2020
@typescript-bot
Copy link
Copy Markdown
Contributor

typescript-bot commented Aug 10, 2020 •
edited
Loading

@reubenrybnik Thank you for submitting this PR!

This is a live comment which I will keep updated.

Code Reviews

Because you edited one package and updated the tests (👏), I can help you merge this PR once someone else signs off on it.

Status

  • ✅ No merge conflicts
  • ✅ Continuous integration tests have passed
  • ✅ Most recent commit is approved by type definition owners or DT maintainers

All of the items on the list are green. To merge, you need to post a comment including the string "Ready to merge" to bring in your changes.

Diagnostic Information: What the bot saw about this PR 
{
  "type": "info",
  "now": "-",
  "pr_number": 46650,
  "author": "reubenrybnik",
  "owners": [
    "borisyankov",
    "jbaldwin",
    "ccurrens",
    "confususs",
    "jgonggrijp",
    "ffflorian",
    "regevbr",
    "peterblazejewicz",
    "reubenrybnik"
  ],
  "dangerLevel": "ScopedAndTested",
  "headCommitAbbrOid": "3653a84",
  "headCommitOid": "3653a84e478198567e1cbffc9029ca020759e0b5",
  "mergeIsRequested": true,
  "stalenessInDays": 0,
  "lastPushDate": "2020-08-10T05:45:42.000Z",
  "lastCommentDate": "2020-08-11T20:39:30.000Z",
  "maintainerBlessed": false,
  "reviewLink": "https://github.com/DefinitelyTyped/DefinitelyTyped/pull/46650/files",
  "hasMergeConflict": false,
  "authorIsOwner": true,
  "isFirstContribution": false,
  "popularityLevel": "Popular",
  "anyPackageIsNew": false,
  "packages": [
    "underscore"
  ],
  "files": [
    {
      "path": "types/underscore/index.d.ts",
      "kind": "definition",
      "package": "underscore"
    },
    {
      "path": "types/underscore/underscore-tests.ts",
      "kind": "test",
      "package": "underscore"
    }
  ],
  "hasDismissedReview": false,
  "ciResult": "pass",
  "lastReviewDate": "2020-08-11T20:02:32.000Z",
  "reviewersWithStaleReviews": [],
  "approvalFlags": 2,
  "isChangesRequested": false
}

@typescript-bot
Copy link
Copy Markdown
Contributor

typescript-bot commented Aug 10, 2020

🔔 @borisyankov @jbaldwin @ccurrens @confususs @jgonggrijp @ffflorian @regevbr @peterblazejewicz — 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.

jgonggrijp
jgonggrijp approved these changes Aug 11, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@jgonggrijp jgonggrijp left a comment

Choose a reason for hiding this comment

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

Fine by me!

@typescript-bot typescript-bot added Owner Approved A listed owner of this package signed off on the pull request. Self Merge This PR can now be self-merged by the PR author or an owner labels Aug 11, 2020
@typescript-bot
Copy link
Copy Markdown
Contributor

typescript-bot commented Aug 11, 2020

@reubenrybnik Everything looks good here. Great job! I am ready to merge this PR on your behalf.

If you'd like that to happen, please post a comment saying:

Ready to merge

and I'll merge this PR almost instantly. Thanks for helping out! ❤️

(@borisyankov, @jbaldwin, @ccurrens, @confususs, @jgonggrijp, @ffflorian, @regevbr, @peterblazejewicz: you can do this too.)

@reubenrybnik
Copy link
Copy Markdown
Contributor Author

reubenrybnik commented Aug 11, 2020

Ready to merge

@typescript-bot typescript-bot merged commit aba9d5f into DefinitelyTyped:master Aug 11, 2020
@typescript-bot
Copy link
Copy Markdown
Contributor

typescript-bot commented Aug 11, 2020

I just published @types/underscore@1.10.21 to npm.

@reubenrybnik reubenrybnik mentioned this pull request Aug 15, 2020
Merged
5 tasks
chivesrs pushed a commit to chivesrs/DefinitelyTyped that referenced this pull request Sep 2, 2020
@reubenrybnik @chivesrs
🤖 Merge PR DefinitelyTyped#46650 [@types/underscore] Support readonly…
0bce410 
… tuples in object calls by @reubenrybnik
danielrearden pushed a commit to danielrearden/DefinitelyTyped that referenced this pull request Sep 22, 2020
@reubenrybnik @danielrearden
🤖 Merge PR DefinitelyTyped#46650 [@types/underscore] Support readonly…
4a4452e 
… tuples in object calls by @reubenrybnik
@reubenrybnik reubenrybnik mentioned this pull request Sep 25, 2020
Closed
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@borisyankov borisyankov Awaiting requested review from borisyankov borisyankov is a code owner

1 more reviewer

@jgonggrijp jgonggrijp jgonggrijp approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Author is Owner The author of this PR is a listed owner of the package. Owner Approved A listed owner of this package signed off on the pull request. Popular package This PR affects a popular package (as counted by NPM download counts). Self Merge This PR can now be self-merged by the PR author or an owner

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@reubenrybnik @typescript-bot @jgonggrijp