Skip to content

[ramda] Improve Tuple definition#59384

Closed
vzhou842 wants to merge 1 commit intoDefinitelyTyped:masterfrom
vzhou842:ramda/tuple
Closed

[ramda] Improve Tuple definition#59384
vzhou842 wants to merge 1 commit intoDefinitelyTyped:masterfrom
vzhou842:ramda/tuple

Conversation

@vzhou842
Copy link
Copy Markdown
Contributor

@vzhou842 vzhou842 commented Mar 22, 2022

Please fill in this template.

Select one of these and delete the others:

If changing an existing definition:

  • Provide a URL to documentation or source code which provides context for the suggested changes: TS playground

This implementation was adapted from this StackOverflow answer.

@typescript-bot
Copy link
Copy Markdown
Contributor

typescript-bot commented Mar 22, 2022 •
edited
Loading

@vzhou842 Thank you for submitting this PR! I see this is your first time submitting to DefinitelyTyped 👋 — I'm the local bot who will help you through the process of getting things through.

This is a live comment which I will keep updated.

1 package in this PR

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.

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

Status

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

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": 59384,
  "author": "vzhou842",
  "headCommitOid": "1f561d856590c10ed707e52299b4b6c70c8f7076",
  "mergeBaseOid": "1eceda21bc57921c609f5fd9f34b2cc8eef3c1f3",
  "lastPushDate": "2022-03-22T02:30:27.000Z",
  "lastActivityDate": "2022-03-22T02:36:47.000Z",
  "hasMergeConflict": false,
  "isFirstContribution": true,
  "tooManyFiles": false,
  "hugeChange": false,
  "popularityLevel": "Popular",
  "pkgInfo": [
    {
      "name": "ramda",
      "kind": "edit",
      "files": [
        {
          "path": "types/ramda/test/aperture-tests.ts",
          "kind": "test"
        },
        {
          "path": "types/ramda/tools.d.ts",
          "kind": "definition"
        }
      ],
      "owners": [
        "TheHandsomeCoder",
        "donnut",
        "mdekrey",
        "sbking",
        "afharo",
        "teves-castro",
        "hojberg",
        "samsonkeung",
        "angeloocana",
        "raynerd",
        "moshensky",
        "ethanresnick",
        "deftomat",
        "blimusiek",
        "biern",
        "rayhaneh",
        "rgm",
        "drewwyatt",
        "jottenlips",
        "minitesh",
        "krantisinh",
        "nemo108",
        "jituanlin",
        "Philippe-mills",
        "Saul-Mirone",
        "Nicholaiii",
        "devrelm",
        "LORDBABUINO",
        "couzic",
        "NEWESTERS",
        "adispring",
        "essenmitsosse"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Popular"
    }
  ],
  "reviews": [],
  "mainBotCommentID": 1074662108,
  "ciResult": "pass"
}

@typescript-bot typescript-bot added the Popular package This PR affects a popular package (as counted by NPM download counts). label Mar 22, 2022
@typescript-bot
Copy link
Copy Markdown
Contributor

typescript-bot commented Mar 22, 2022

🔔 @TheHandsomeCoder @donnut @mdekrey @sbking @afharo @teves-castro @hojberg @samsonkeung @angeloocana @raynerd @moshensky @ethanresnick @deftomat @blimusiek @biern @rayhaneh @rgm @drewwyatt @jottenlips @minitesh @Krantisinh @Nemo108 @jituanlin @Philippe-mills @Saul-Mirone @Nicholaiii @devrelm @LORDBABUINO @couzic @NEWESTERS @adispring @essenmitsosse — 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.

@DangerBotOSS
Copy link
Copy Markdown

DangerBotOSS commented Mar 22, 2022

Inspecting the JavaScript source for this package found some properties that are not in the .d.ts files.
The check for missing properties isn't always right, so take this list as advice, not a requirement.

ramda (unpkg)

was missing the following properties:

  1. collectBy
  2. modify
  3. modifyPath
  4. partialObject
  5. on
as well as these 5 other properties...

promap, sequence, splitWhenever, unwind, whereAny

Generated by 🚫 dangerJS against 1f561d8

@vzhou842
Copy link
Copy Markdown
Contributor Author

vzhou842 commented Mar 22, 2022 •
edited
Loading

ok never mind, there's a notable flaw with this approach: it doesn't work if T is any, never, null, or undefined. These 4 types are all assignable to undefined, meaning they all extend undefined. The implementation proposed in this PRs depends on the faulty assumption that only undefined extends undefined

@vzhou842 vzhou842 closed this Mar 22, 2022
@vzhou842 vzhou842 mentioned this pull request Mar 22, 2022
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Popular package This PR affects a popular package (as counted by NPM download counts).

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vzhou842 @typescript-bot @DangerBotOSS