[express-serve-static-core] fix stack depth error on long path parameters #54151
Conversation
|
@DetachHead Thank you for submitting this PR! This is a live comment which I will keep updated. 1 package in this PRCode ReviewsThis PR 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": 54151,
"author": "DetachHead",
"headCommitOid": "0f83c2c2d6b0848fadb01eb75d1e39fa5aeab16a",
"lastPushDate": "2021-07-03T03:24:25.000Z",
"lastActivityDate": "2021-07-08T14:19:43.000Z",
"maintainerBlessed": "Waiting for Author to Merge",
"mergeOfferDate": "2021-07-08T13:39:51.000Z",
"mergeRequestDate": "2021-07-08T14:19:43.000Z",
"mergeRequestUser": "DetachHead",
"hasMergeConflict": false,
"isFirstContribution": false,
"tooManyFiles": false,
"popularityLevel": "Critical",
"pkgInfo": [
{
"name": "express-serve-static-core",
"kind": "edit",
"files": [
{
"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"
}
],
"owners": [
"borisyankov",
"19majkel94",
"kacepe",
"micksatana",
"samijaber",
"JoseLion",
"dwrss",
"andoshin11"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Critical"
}
],
"reviews": [
{
"type": "approved",
"reviewer": "ahejlsberg",
"date": "2021-07-06T16:53:47.000Z",
"isMaintainer": false
}
],
"mainBotCommentID": 870583928,
"ciResult": "pass"
} |
|
🔔 @borisyankov @19majkel94 @kacepe @micksatana @samijaber @JoseLion @dwrss @andoshin11 — please review this PR in the next few days. Be sure to explicitly select |
… `.`/`-` delimited parameters by @DetachHead * specify minimum ts version for template literal types * set minimum ts version for absolute-url to 4.1 since it depends on express which now requires >= 4.1 * set minimum typescript version for express-serve-static-core to 4.1 * revert changes to minimum ts version, gonna make a seperate <=4.0 package instead * support optional params and params separated by dashes and dots * oops tests in the wrong spot * fallback to ParamsDictionary when regex is detected in route params * use ParamsDictionary when string extends Route * more descriptive generic names * strict-export-declare-modifiers lint error * prettier fix * improve formatting for RouteParameters type Co-authored-by: Jose Luis Leon <[email protected]> * prettier-ignore RouteParameters type * change never in RouteParameters condition to unknown Co-authored-by: Ron Buckton <[email protected]> Co-authored-by: DetachHead <[email protected]> Co-authored-by: Jose Luis Leon <[email protected]> Co-authored-by: Ron Buckton <[email protected]>
|
@ahejlsberg 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? |
|
Any word on this? This is a super-blocker for updating packages right now. |
typescript-bot
added
Other Approved
|
@DetachHead: Everything looks good here. I am ready to merge this PR (at 0f83c2c) on your behalf whenever you think it's ready. If you'd like that to happen, please post a comment saying:
and I'll merge this PR almost instantly. Thanks for helping out! ❤️ (@borisyankov, @19majkel94, @kacepe, @micksatana, @samijaber, @JoseLion, @dwrss, @andoshin11: you can do this too.) |
|
Ready to merge |
|
I just published |
4 participants
Please fill in this template.
fix for this issue caused by my previous PR: #53513 (comment)
i'm not really sure if this is the best way to detect the stack depth error. open to suggestions
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>.Provide a URL to documentation or source code which provides context for the suggested changes: [express-serve-static-core] optional parameters and
./-delimited parameters #53513 (comment)If this PR brings the type definitions up to date with a new version of the JS library, update the version number in the header.