Conversation
|
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
|
Hey @RobinTail, can you review again please? I implemented your suggestions. |
Co-authored-by: Anna Bocharova <robin_tail@me.com>
|
Can we release this as a minor and recommend users to migrate away from |
|
Nope, this should be released in a major version, mainly because it could break TypeScript users, whether they use it with Express or not. Besides, it's not natural to add TypeScript support in a non-major version. |
Phillip9587
left a comment
Phillip9587
left a comment
There was a problem hiding this comment.
How about merging this into the master branch without publishing it for now?
That way, we can continue iterating and improving the types.
Once we're ready to release v3, we can add index.d.ts to the files and types fields in package.json.
| "repository": "pillarjs/finalhandler", | ||
| "type": "commonjs", | ||
| "main": "index.js", | ||
| "types": "index.d.ts", |
There was a problem hiding this comment.
| "types": "index.d.ts", |
There was a problem hiding this comment.
I believe types would be resolved automatically even without this entry.
Just because of the index.* naming
There was a problem hiding this comment.
At least in my PR attw resolves them without types in package.json
| "index.js", | ||
| "index.d.ts" |
There was a problem hiding this comment.
| "index.js", | |
| "index.d.ts" | |
| "index.js" |
|
Just to say, it's normal to release this in a minor version. Also, this is blocked until the types are officially accepted in the repositories. |
|
on my todo for review |
3 participants
This PR adds type definitions to
finalhandler. It includes the following changes:@types/finalhandlerinto the repository (https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/finalhandler)type,mainandtypesfield topackage.jsontsconfig.jsonwhich extends@tsconfig/node18from https://github.com/tsconfig/bases@arethetypeswrong/cliandtsdRef: expressjs/typescript-wg#1
cc @RobinTail @hichemfantar