feat: types for express 5#70563
Conversation
|
✅ All tests pass now.
I'm requesting comments and I'm open for advises and suggestions on how this important change could be made even better and released faster. |
|
@RobinTail Thank you for submitting this PR! This is a live comment that I will keep updated. This PR touches some part of DefinitelyTyped infrastructure, so a DT maintainer will need to review it. This is rare — did you mean to do this? 16 packages in this PR (and infra files)
Code ReviewsBecause 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
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": 70563,
"author": "RobinTail",
"headCommitOid": "8f3a52d98bbbe1539d19bda7cc63581c241d4d9b",
"mergeBaseOid": "b0208c9d870b7bfa747d7ed9c7c108c95207cc97",
"lastPushDate": "2024-09-14T08:37:10.000Z",
"lastActivityDate": "2024-09-25T19:08:01.000Z",
"mergeOfferDate": "2024-09-25T17:37:04.000Z",
"mergeRequestDate": "2024-09-25T19:08:01.000Z",
"mergeRequestUser": "RobinTail",
"hasMergeConflict": false,
"isFirstContribution": false,
"tooManyFiles": false,
"hugeChange": false,
"popularityLevel": "Critical",
"pkgInfo": [
{
"name": null,
"kind": "edit",
"files": [
{
"path": "attw.json",
"kind": "infrastructure"
}
],
"owners": [],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Critical",
"isSafeInfrastructureEdit": false
},
{
"name": "absolute-url",
"kind": "edit",
"files": [
{
"path": "types/absolute-url/package.json",
"kind": "package-meta-ok"
}
],
"owners": [
"tpluscode",
"ludovicm67"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Well-liked by everyone"
},
{
"name": "architect__functions",
"kind": "edit",
"files": [
{
"path": "types/architect__functions/test/http-tests.ts",
"kind": "test"
}
],
"owners": [
"activescott",
"ryanblock",
"reconbot"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Well-liked by everyone"
},
{
"name": "create-test-server",
"kind": "edit",
"files": [
{
"path": "types/create-test-server/index.d.ts",
"kind": "definition"
}
],
"owners": [
"midgleyc"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Well-liked by everyone"
},
{
"name": "express-brute",
"kind": "edit",
"files": [
{
"path": "types/express-brute/index.d.ts",
"kind": "definition"
}
],
"owners": [
"cyrilschumacher"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Well-liked by everyone"
},
{
"name": "express-oauth-server",
"kind": "edit",
"files": [
{
"path": "types/express-oauth-server/express-oauth-server-tests.ts",
"kind": "test"
},
{
"path": "types/express-oauth-server/index.d.ts",
"kind": "definition"
}
],
"owners": [
"atd-schubert"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Well-liked by everyone"
},
{
"name": "express-serve-static-core",
"kind": "edit",
"files": [
{
"path": "types/express-serve-static-core/.npmignore",
"kind": "package-meta-ok"
},
{
"path": "types/express-serve-static-core/express-serve-static-core-tests.ts",
"kind": "test"
},
{
"path": "types/express-serve-static-core/index.d.ts",
"kind": "definition"
},
{
"path": "types/express-serve-static-core/package.json",
"kind": "package-meta-ok"
},
{
"path": "types/express-serve-static-core/v4/.eslintrc.json",
"kind": "package-meta",
"suspect": "edited"
},
{
"path": "types/express-serve-static-core/v4/.npmignore",
"kind": "package-meta-ok"
},
{
"path": "types/express-serve-static-core/v4/express-serve-static-core-tests.ts",
"kind": "test"
},
{
"path": "types/express-serve-static-core/v4/index.d.ts",
"kind": "definition"
},
{
"path": "types/express-serve-static-core/v4/package.json",
"kind": "package-meta-ok"
},
{
"path": "types/express-serve-static-core/v4/tsconfig.json",
"kind": "package-meta-ok"
}
],
"owners": [
"borisyankov",
"micksatana",
"JoseLion",
"dwrss",
"andoshin11"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Critical"
},
{
"name": "express-ua-middleware",
"kind": "edit",
"files": [
{
"path": "types/express-ua-middleware/index.d.ts",
"kind": "definition"
}
],
"owners": [
"peterblazejewicz"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Well-liked by everyone"
},
{
"name": "express",
"kind": "edit",
"files": [
{
"path": "types/express/.npmignore",
"kind": "package-meta-ok"
},
{
"path": "types/express/express-tests.ts",
"kind": "test"
},
{
"path": "types/express/package.json",
"kind": "package-meta-ok"
},
{
"path": "types/express/v4/.eslintrc.json",
"kind": "package-meta",
"suspect": "edited"
},
{
"path": "types/express/v4/.npmignore",
"kind": "package-meta-ok"
},
{
"path": "types/express/v4/express-tests.ts",
"kind": "test"
},
{
"path": "types/express/v4/index.d.ts",
"kind": "definition"
},
{
"path": "types/express/v4/package.json",
"kind": "package-meta-ok"
},
{
"path": "types/express/v4/tsconfig.json",
"kind": "package-meta-ok"
}
],
"owners": [
"borisyankov",
"CMUH",
"puneetar",
"dfrankland"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Critical"
},
{
"name": "feathersjs__express",
"kind": "edit",
"files": [
{
"path": "types/feathersjs__express/feathersjs__express-tests.ts",
"kind": "test"
}
],
"owners": [
"j2L4e",
"DadUndead"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Well-liked by everyone"
},
{
"name": "forest-express-mongoose",
"kind": "edit",
"files": [
{
"path": "types/forest-express-mongoose/forest-express-mongoose-tests.ts",
"kind": "test"
}
],
"owners": [
"SteveBunlon",
"ghusse"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Well-liked by everyone"
},
{
"name": "forest-express-sequelize",
"kind": "edit",
"files": [
{
"path": "types/forest-express-sequelize/forest-express-sequelize-tests.ts",
"kind": "test"
}
],
"owners": [
"SteveBunlon",
"ghusse"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Well-liked by everyone"
},
{
"name": "fusebit__oauth-connector",
"kind": "edit",
"files": [
{
"path": "types/fusebit__oauth-connector/fusebit__oauth-connector-tests.ts",
"kind": "test"
}
],
"owners": [
"andrewrmiller",
"hashiphoto",
"andydam"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Well-liked by everyone"
},
{
"name": "logfmt",
"kind": "edit",
"files": [
{
"path": "types/logfmt/logfmt-tests.ts",
"kind": "test"
}
],
"owners": [
"ebroder"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Well-liked by everyone"
},
{
"name": "mock-req-res",
"kind": "edit",
"files": [
{
"path": "types/mock-req-res/mock-req-res-tests.ts",
"kind": "test"
}
],
"owners": [
"sandorTuranszky"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Well-liked by everyone"
},
{
"name": "mongoose-aggregate-paginate-v2",
"kind": "edit",
"files": [
{
"path": "types/mongoose-aggregate-paginate-v2/mongoose-aggregate-paginate-v2-tests.ts",
"kind": "test"
}
],
"owners": [
"acrilex1"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Well-liked by everyone"
},
{
"name": "swaggerize-express",
"kind": "edit",
"files": [
{
"path": "types/swaggerize-express/swaggerize-express-tests.ts",
"kind": "test"
}
],
"owners": [
"mugeso",
"nickmorton"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Well-liked by everyone"
}
],
"reviews": [
{
"type": "approved",
"reviewer": "jakebailey",
"date": "2024-09-25T17:36:23.000Z",
"isMaintainer": true
},
{
"type": "stale",
"reviewer": "ludovicm67",
"date": "2024-09-23T11:46:42.000Z",
"abbrOid": "635727b"
},
{
"type": "stale",
"reviewer": "midgleyc",
"date": "2024-09-23T10:44:45.000Z",
"abbrOid": "635727b"
},
{
"type": "stale",
"reviewer": "tpluscode",
"date": "2024-09-16T12:34:58.000Z",
"abbrOid": "635727b"
}
],
"mainBotCommentID": 2350955911,
"ciResult": "pass"
} |
|
🔔 @tpluscode @ludovicm67 @activescott @ryanblock @reconbot @midgleyc @cyrilschumacher @atd-schubert @borisyankov @micksatana @JoseLion @dwrss @andoshin11 @peterblazejewicz @CMUH @puneetar @dfrankland @j2L4e @DadUndead @SteveBunlon @ghusse @andrewrmiller @Hashiphoto @andydam @ebroder @sandorTuranszky @acrilex1 @MugeSo @nickmorton — please review this PR in the next few days. Be sure to explicitly select |
|
the rule is only included in I realize I can disable lint for that line/file, cast the type, or disable rule. The thing is... I believe the rule is correct. The express handler possibly returns a Promise (according to the type), and node http handler doesn't handle promises. Just an FYI, since it might be a common use-case to use the app itself as handler for a native http server. Not sure I want to tackle this on my own. I'll probably just workaround in user-end. |
|
There seems to be a version management discrepancy. While Current package.json: {
"dependencies": {
"express": "^5.0.1"
},
"devDependencies": {
"@types/express": "^5.0.0"
}
}Running This creates potential confusion since:
|
|
That was an intentional choice made by the express maintainers; v5 is not marked as latest over there. DefinitelyTyped has no mechanism to model this somewhat unusual situation. |
|
I see, thanks for the clarification @jakebailey I'll just disable |
…nTail Co-authored-by: Jake Bailey <[email protected]>
@AviVahl Have you figured out a workaround yet? (Experiencing the same issue here) |
use a cast: import { createServer, type RequestListener } from "node:http";
import express from "express";
const app = express();
const httpServer = createServer(app as RequestListener);or use |
|
Thanks, @AviVahl, I'll do this for now, temporarily. Using express@5 here. Although, force casting should be avoided. Wondering if this has every been raised upstream before and will ever be fixed? |
|
I ran into the same issue with ESLint |
Charset is sent in the content-type (expressjs/express@a0276c6). @types/express now accepts promises (DefinitelyTyped/DefinitelyTyped#70563). New `dotfiles` options for serve-static. See <https://github.com/expressjs/serve-static/releases/tag/v2.0.0-beta.1>.
Charset is sent in the content-type (expressjs/express@a0276c6). @types/express now accepts promises (DefinitelyTyped/DefinitelyTyped#70563). New `dotfiles` options for serve-static. See <https://github.com/expressjs/serve-static/releases/tag/v2.0.0-beta.1>.
Charset is sent in the content-type (expressjs/express@a0276c6). @types/express now accepts promises (DefinitelyTyped/DefinitelyTyped#70563). New `dotfiles` options for serve-static. See <https://github.com/expressjs/serve-static/releases/tag/v2.0.0-beta.1>.
Alternative to #69846 (comment) — the author is sick and can not continue.
The primary goal of this PR is to release the types for
expressversion 5 — the main difference is allowingPromise<void>along withvoidonRequestHandler.Challenge:
voidwas technically compatible with anything actually returned, butvoid | Promise<void>is not.Tracked in: expressjs/express#5944
Please fill in this template.
pnpm test <package to test>.Select one of these and delete the others:
If adding a new definition:
.d.tsfiles generated via--declarationdts-gen --dt, not by basing it on an existing project.tsconfig.jsonshould havenoImplicitAny,noImplicitThis,strictNullChecks, andstrictFunctionTypesset totrue.If changing an existing definition:
package.json.If removing a declaration:
notNeededPackages.json.