add `function.prototype.name` #66734

ljharb · 2023-09-18T06:01:52Z

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 npm test <package to test>.

Select one of these and delete the others:

If adding a new definition:

The package does not already provide its own types, or cannot have its .d.ts files generated via --declaration
If this is for an npm package, match the name. If not, do not conflict with the name of an npm package.
Create it with dts-gen --dt, not by basing it on an existing project.
Represents shape of module/library correctly
tslint.json should contain { "extends": "@definitelytyped/dtslint/dt.json" }, and no additional rules.
tsconfig.json should have noImplicitAny, noImplicitThis, strictNullChecks, and strictFunctionTypes set to true.

typescript-bot · 2023-09-18T06:02:36Z

@ljharb Thank you for submitting this PR!

This is a live comment which I will keep updated.

1 package in this PR

function.prototype.name (new!) — on npm, on unpkg
- 1 added owner: ✎@ljharb
- Config files to check:
  - function.prototype.name/tsconfig.json: not the expected form (check: compilerOptions.esModuleInterop)

Code Reviews

This PR adds a new definition, so it needs to be reviewed by a DT maintainer before it can be merged.

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

Status

✅ No merge conflicts
✅ Continuous integration tests have passed
✅ Only a DT maintainer can approve changes when there are new packages added

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": 66734,
  "author": "ljharb",
  "headCommitOid": "521d0a3c41bc94502be878611ab8955de83fd065",
  "mergeBaseOid": "3a2e6cbf0c40245af9f02b5011da2ea713cd351d",
  "lastPushDate": "2023-09-18T06:01:52.000Z",
  "lastActivityDate": "2023-09-26T23:22:08.000Z",
  "mergeOfferDate": "2023-09-26T19:59:42.000Z",
  "mergeRequestDate": "2023-09-26T23:22:08.000Z",
  "mergeRequestUser": "ljharb",
  "hasMergeConflict": false,
  "isFirstContribution": false,
  "tooManyFiles": false,
  "hugeChange": false,
  "popularityLevel": "Well-liked by everyone",
  "pkgInfo": [
    {
      "name": "function.prototype.name",
      "kind": "add",
      "files": [
        {
          "path": "types/function.prototype.name/auto.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/function.prototype.name/function.prototype.name-tests.ts",
          "kind": "test"
        },
        {
          "path": "types/function.prototype.name/implementation.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/function.prototype.name/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/function.prototype.name/polyfill.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/function.prototype.name/shim.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/function.prototype.name/tsconfig.json",
          "kind": "package-meta",
          "suspect": "not [the expected form](https://github.com/DefinitelyTyped/DefinitelyTyped#user-content-tsconfigjson) (check: `compilerOptions.esModuleInterop`)"
        },
        {
          "path": "types/function.prototype.name/tslint.json",
          "kind": "package-meta-ok"
        }
      ],
      "owners": [],
      "addedOwners": [
        "ljharb"
      ],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    }
  ],
  "reviews": [
    {
      "type": "approved",
      "reviewer": "RyanCavanaugh",
      "date": "2023-09-26T19:58:55.000Z",
      "isMaintainer": true
    },
    {
      "type": "stale",
      "reviewer": "sheetalkamat",
      "date": "2023-09-22T18:35:08.000Z",
      "abbrOid": "1167a14"
    }
  ],
  "mainBotCommentID": 1722798192,
  "ciResult": "pass"
}

typescript-bot · 2023-09-18T06:02:39Z

🔔 @ljharb — you're the only owner, but it would still be good if you find someone to review this PR in the next few days, otherwise a maintainer will look at it. (And if you do find someone, maybe even recruit them to be a second owner to make future changes easier...)

typescript-bot · 2023-09-18T06:04:24Z

@ljharb 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 which are failing do not end up on the list of PRs for the DT maintainers to review.

typescript-bot · 2023-09-18T06:08:35Z

@ljharb 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 which are failing do not end up on the list of PRs for the DT maintainers to review.

typescript-bot · 2023-09-18T06:15:21Z

@ljharb 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 which are failing do not end up on the list of PRs for the DT maintainers to review.

sheetalkamat · 2023-09-18T21:52:06Z

types/function.prototype.name/implementation.d.ts

+type Func = (...args: any[]) => any;
+
+declare function name(): (this: Func) => string;
+
+export = name;


I dont think you need all these files. you just need index.d.ts since rest are implementation details and not needed in types package

They are not implementation details; they're all explicit entry points that I use in dozens of packages, and are present in exports.

Rebased and repushed so the bot will do a proper rereview ping.

Do note that I have a hundred packages with this structure; some already are on DT; all of those have this DT file format already - so this is already an approved and accepted approach.

https://github.com/es-shims/Function.prototype.name#example here talks about importing function.prototype.name so i dont understand why others are explicit entry points. It seems like these definitions could be simplified into index.d.ts

That's not how my entire ecosystem of shims works. The example only talks about that, but all of my shims have the same 4 entry points.

It feels like you're arguing about the design of my library, and not about the quality of the types. This is how my library is designed, this is how the types need to be for consumers to use it correctly. I'm not sure why this is up for debate?

Here's a few existing PRs where this exact (proper) pattern was followed:

Add promise.any #53498

Add types for es-aggregate-error #52716

add object.fromentries #44406

Rebased this again to make sure it shows up in the right queue.

typescript-bot · 2023-09-21T18:28:59Z

@ljharb 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 · 2023-09-21T21:59:09Z

@sheetalkamat Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

sheetalkamat · 2023-09-22T18:35:05Z

types/function.prototype.name/implementation.d.ts

+type Func = (...args: any[]) => any;
+
+declare function name(): (this: Func) => string;
+
+export = name;


https://github.com/es-shims/Function.prototype.name#example here talks about importing function.prototype.name so i dont understand why others are explicit entry points. It seems like these definitions could be simplified into index.d.ts

typescript-bot · 2023-09-22T18:35:54Z

@ljharb 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 · 2023-09-25T10:40:19Z

@sheetalkamat Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

typescript-bot · 2023-09-26T19:59:42Z

@ljharb: Everything looks good here. I am ready to merge this PR (at 521d0a3) on your behalf whenever you think it's ready.

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! ❤️

ljharb · 2023-09-26T23:22:08Z

Ready to merge

add function.prototype.name

521d0a3

typescript-bot added New Definition This PR creates a new definition package. Check Config Changes a module config files labels Sep 18, 2023

typescript-bot added the The CI failed When GH Actions fails label Sep 18, 2023

ljharb force-pushed the function.prototype.name branch from d320dd4 to e80ee56 Compare September 18, 2023 06:06

typescript-bot removed the The CI failed When GH Actions fails label Sep 18, 2023

typescript-bot added the The CI failed When GH Actions fails label Sep 18, 2023

ljharb force-pushed the function.prototype.name branch from e80ee56 to 062bc18 Compare September 18, 2023 06:09

typescript-bot removed the The CI failed When GH Actions fails label Sep 18, 2023

typescript-bot added the The CI failed When GH Actions fails label Sep 18, 2023

ljharb force-pushed the function.prototype.name branch from 062bc18 to 16c1d13 Compare September 18, 2023 06:15

typescript-bot removed the The CI failed When GH Actions fails label Sep 18, 2023

sheetalkamat requested changes Sep 21, 2023

View reviewed changes

typescript-bot added the Revision needed This PR needs code changes before it can be merged. label Sep 21, 2023

ljharb force-pushed the function.prototype.name branch from 16c1d13 to 1167a14 Compare September 21, 2023 21:54

typescript-bot removed the Revision needed This PR needs code changes before it can be merged. label Sep 21, 2023

sheetalkamat requested changes Sep 22, 2023

View reviewed changes

typescript-bot added the Revision needed This PR needs code changes before it can be merged. label Sep 22, 2023

ljharb force-pushed the function.prototype.name branch from 1167a14 to 521d0a3 Compare September 25, 2023 10:34

typescript-bot removed the Revision needed This PR needs code changes before it can be merged. label Sep 25, 2023

ljharb requested a review from sheetalkamat September 25, 2023 12:01

RyanCavanaugh approved these changes Sep 26, 2023

View reviewed changes

typescript-bot added Maintainer Approved Self Merge This PR can now be self-merged by the PR author or an owner labels Sep 26, 2023

typescript-bot merged commit e95a640 into DefinitelyTyped:master Sep 26, 2023

ljharb deleted the function.prototype.name branch September 26, 2023 23:24

ekwoka mentioned this pull request Oct 1, 2023

📝 Improves Documentation of Alpine Methods #66890

Closed

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add `function.prototype.name` #66734

add `function.prototype.name` #66734

ljharb commented Sep 18, 2023

typescript-bot commented Sep 18, 2023 •

edited

Loading

typescript-bot commented Sep 18, 2023

typescript-bot commented Sep 18, 2023

typescript-bot commented Sep 18, 2023

typescript-bot commented Sep 18, 2023

sheetalkamat Sep 18, 2023

ljharb Sep 21, 2023

ljharb Sep 21, 2023

sheetalkamat Sep 22, 2023

ljharb Sep 22, 2023

ljharb Sep 23, 2023

ljharb Sep 25, 2023

typescript-bot commented Sep 21, 2023

typescript-bot commented Sep 21, 2023

sheetalkamat Sep 22, 2023

typescript-bot commented Sep 22, 2023

typescript-bot commented Sep 25, 2023

typescript-bot commented Sep 26, 2023

ljharb commented Sep 26, 2023

add function.prototype.name #66734

add function.prototype.name #66734

Conversation

ljharb commented Sep 18, 2023

typescript-bot commented Sep 18, 2023 • edited Loading

1 package in this PR

Code Reviews

Status

typescript-bot commented Sep 18, 2023

typescript-bot commented Sep 18, 2023

typescript-bot commented Sep 18, 2023

typescript-bot commented Sep 18, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

typescript-bot commented Sep 21, 2023

typescript-bot commented Sep 21, 2023

Choose a reason for hiding this comment

typescript-bot commented Sep 22, 2023

typescript-bot commented Sep 25, 2023

typescript-bot commented Sep 26, 2023

ljharb commented Sep 26, 2023

add `function.prototype.name` #66734

add `function.prototype.name` #66734

typescript-bot commented Sep 18, 2023 •

edited

Loading