New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(node): declare URL and URLSearchParams as globals #58277
feat(node): declare URL and URLSearchParams as globals #58277
Conversation
@antongolub Thank you for submitting this PR! This is a live comment which I will keep updated. 1 package in this PR
Code ReviewsBecause 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
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": 58277,
"author": "antongolub",
"headCommitOid": "5f9bcb8fe3c2f9aedc05c9dadc20ee7157f14838",
"mergeBaseOid": "ba9c5ca17fd39d9e7e3ce89be9fafd6b1bc9e24f",
"lastPushDate": "2022-01-20T13:03:36.000Z",
"lastActivityDate": "2022-02-01T08:07:38.000Z",
"maintainerBlessed": "Waiting for Code Reviews",
"mergeOfferDate": "2022-01-31T20:07:31.000Z",
"mergeRequestDate": "2022-02-01T08:07:38.000Z",
"mergeRequestUser": "antongolub",
"hasMergeConflict": false,
"isFirstContribution": false,
"tooManyFiles": false,
"hugeChange": false,
"popularityLevel": "Critical",
"pkgInfo": [
{
"name": "node",
"kind": "edit",
"files": [
{
"path": "types/node/test/url.ts",
"kind": "test"
},
{
"path": "types/node/url.d.ts",
"kind": "definition"
},
{
"path": "types/node/v12/url.d.ts",
"kind": "definition"
},
{
"path": "types/node/v14/test/url.ts",
"kind": "test"
},
{
"path": "types/node/v14/url.d.ts",
"kind": "definition"
},
{
"path": "types/node/v16/test/url.ts",
"kind": "test"
},
{
"path": "types/node/v16/url.d.ts",
"kind": "definition"
}
],
"owners": [
"Microsoft",
"DefinitelyTyped",
"jkomyno",
"alvis",
"r3nya",
"btoueg",
"smac89",
"touffy",
"DeividasBakanas",
"eyqs",
"Hannes-Magnusson-CK",
"hoo29",
"kjin",
"ajafff",
"islishude",
"mwiktorczyk",
"mohsen1",
"n-e",
"galkin",
"parambirs",
"eps1lon",
"westy92",
"SimonSchick",
"ThomasdenH",
"WilcoBakker",
"wwwy3y3",
"samuela",
"kuehlein",
"bhongy",
"chyzwar",
"trivikr",
"yoursunny",
"qwelias",
"ExE-Boss",
"peterblazejewicz",
"addaleax",
"victorperin",
"ZYSzys",
"NodeJS",
"LinusU",
"wafuwafu13"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Critical"
}
],
"reviews": [
{
"type": "approved",
"reviewer": "peterblazejewicz",
"date": "2022-01-31T20:06:52.000Z",
"isMaintainer": true
}
],
"mainBotCommentID": 1015556159,
"ciResult": "pass"
} |
🔔 @microsoft @DefinitelyTyped @jkomyno @alvis @r3nya @btoueg @smac89 @Touffy @DeividasBakanas @eyqs @Hannes-Magnusson-CK @hoo29 @kjin @ajafff @islishude @mwiktorczyk @mohsen1 @n-e @galkin @parambirs @eps1lon @westy92 @SimonSchick @ThomasdenH @WilcoBakker @wwwy3y3 @samuela @kuehlein @bhongy @chyzwar @trivikr @yoursunny @qwelias @ExE-Boss @peterblazejewicz @addaleax @victorperin @ZYSzys @nodejs @LinusU @wafuwafu13 — please review this PR in the next few days. Be sure to explicitly select |
Thank you very much for sending a PR. The I am really excited for this fix: it will be very helpful! |
78783ea
to
0ac59f6
Compare
@antongolub 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. |
0ac59f6
to
6a9e8e4
Compare
@antongolub 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. |
TS declare var AbortSignal: {
prototype: AbortSignal;
new(): AbortSignal;
abort(reason?: any): AbortSignal;
}; "version": "4.6.0-dev.20220117" declare var AbortSignal: {
prototype: AbortSignal;
new(): AbortSignal;
// abort(): AbortSignal; - To be re-added in the future
}; UPD |
6a9e8e4
to
3428cc4
Compare
Ready to merge |
Note: This breaks configs that have |
@sandersn what's the option here we can pick? There are two posts on discussion with same (or similar) problem already |
Interesting... createObjectURL(object: any): string;
createObjectURL(obj: Blob | MediaSource): string; more and more interesting: |
Yet another janky trick: var URL:
// For compatibility with "dom" and "webworker" URL declarations
typeof globalThis extends ({ webkitURL: infer URL } | { TransformStreamDefaultController: infer T, URL: infer URL })
? URL
: typeof _URL; @sandersn, could you check this plz? |
afaik it does not work with use case (from your PR branch):
(the DT lib declaring
|
Oops... 3.8 does not provide |
@peterblazejewicz, how about? var URL:
// For compatibility with "dom" and "webworker" URL declarations
typeof globalThis extends { onmessage: infer O, URL: infer URL }
? URL
: typeof _URL; |
Yes, this passes the use-case test, but let's have more opinion on what next. |
… "webworker" by @antongolub * fix(node): make global ULR decl compatible with lib "webworker" relates #58277 #34960 * fix(node): improve global URL var type resolution * refactor(node): handle URLSeachParams var type collision in the same way as for URL * refactor(node): simplify type condition for global URL and URLSearchParams
…hParams as globals by @antongolub * feat(node): declare URL and URLSearchParams as globals closes: DefinitelyTyped#34960 * chore(node): simplify URL and URLSearchParams global definitions * docs(node): add @SInCE comments for URL and URLSearchParam globals * refactor(node): tweak up URL global type after tsc manual tests * docs(node): minor jsdoc improvements for global URL and URLSearchParams
…atible with lib "webworker" by @antongolub * fix(node): make global ULR decl compatible with lib "webworker" relates DefinitelyTyped#58277 DefinitelyTyped#34960 * fix(node): improve global URL var type resolution * refactor(node): handle URLSeachParams var type collision in the same way as for URL * refactor(node): simplify type condition for global URL and URLSearchParams
Anybody that's still watching this, maybe you could weigh in on #59905 where I'm trying to do something similar with another Node "DOM-compatible" API? I'm going to try to use the same pattern you discovered here, if possible. |
closes: #34960
NodeJS spec:
npm test <package to test>
.If changing an existing definition: