-
Notifications
You must be signed in to change notification settings - Fork 30.5k
[node] Update typings to v22.4.0 #70262
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
Merged
typescript-bot
merged 1 commit into
DefinitelyTyped:master
from
Semigradsky:node-22.4.0
Aug 16, 2024
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -103,6 +103,84 @@ declare global { | |
| }; | ||
| // #endregion borrowed | ||
|
|
||
| // #region Storage | ||
| /** | ||
| * This Web Storage API interface provides access to a particular domain's session or local storage. It allows, for example, the addition, modification, or deletion of stored data items. | ||
| * | ||
| * [MDN Reference](https://developer.mozilla.org/docs/Web/API/Storage) | ||
| */ | ||
| interface Storage { | ||
| /** | ||
| * Returns the number of key/value pairs. | ||
| * | ||
| * [MDN Reference](https://developer.mozilla.org/docs/Web/API/Storage/length) | ||
| */ | ||
| readonly length: number; | ||
| /** | ||
| * Removes all key/value pairs, if there are any. | ||
| * | ||
| * Dispatches a storage event on Window objects holding an equivalent Storage object. | ||
| * | ||
| * [MDN Reference](https://developer.mozilla.org/docs/Web/API/Storage/clear) | ||
| */ | ||
| clear(): void; | ||
| /** | ||
| * Returns the current value associated with the given key, or null if the given key does not exist. | ||
| * | ||
| * [MDN Reference](https://developer.mozilla.org/docs/Web/API/Storage/getItem) | ||
| */ | ||
| getItem(key: string): string | null; | ||
| /** | ||
| * Returns the name of the nth key, or null if n is greater than or equal to the number of key/value pairs. | ||
| * | ||
| * [MDN Reference](https://developer.mozilla.org/docs/Web/API/Storage/key) | ||
| */ | ||
| key(index: number): string | null; | ||
| /** | ||
| * Removes the key/value pair with the given key, if a key/value pair with the given key exists. | ||
| * | ||
| * Dispatches a storage event on Window objects holding an equivalent Storage object. | ||
| * | ||
| * [MDN Reference](https://developer.mozilla.org/docs/Web/API/Storage/removeItem) | ||
| */ | ||
| removeItem(key: string): void; | ||
| /** | ||
| * Sets the value of the pair identified by key to value, creating a new key/value pair if none existed for key previously. | ||
| * | ||
| * Throws a "QuotaExceededError" DOMException exception if the new value couldn't be set. (Setting could fail if, e.g., the user has disabled storage for the site, or if the quota has been exceeded.) | ||
| * | ||
| * Dispatches a storage event on Window objects holding an equivalent Storage object. | ||
| * | ||
| * [MDN Reference](https://developer.mozilla.org/docs/Web/API/Storage/setItem) | ||
| */ | ||
| setItem(key: string, value: string): void; | ||
| } | ||
|
|
||
| var Storage: typeof globalThis extends { onmessage: any; Storage: infer T } ? T | ||
| : { | ||
| prototype: Storage; | ||
| new(): Storage; | ||
| }; | ||
|
|
||
| /** | ||
| * A browser-compatible implementation of [`localStorage`](https://developer.mozilla.org/en-US/docs/Web/API/Window/localStorage). | ||
| * Data is stored unencrypted in the file specified by the `--localstorage-file` CLI flag. | ||
| * Any modification of this data outside of the Web Storage API is not supported. | ||
| * Enable this API with the `--experimental-webstorage` CLI flag. | ||
| * @since v22.4.0 | ||
| */ | ||
| var localStorage: Storage; | ||
|
|
||
| /** | ||
| * A browser-compatible implementation of [`sessionStorage`](https://developer.mozilla.org/en-US/docs/Web/API/Window/sessionStorage). | ||
| * Data is stored in memory, with a storage quota of 10 MB. | ||
| * Any modification of this data outside of the Web Storage API is not supported. | ||
| * Enable this API with the `--experimental-webstorage` CLI flag. | ||
| * @since v22.4.0 | ||
| */ | ||
| var sessionStorage: Storage; | ||
|
Comment on lines
+165
to
+181
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should these use the DOM types if available?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| // #endregion Storage | ||
|
|
||
| // #region Disposable | ||
| interface SymbolConstructor { | ||
| /** | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Semigradsky what are you thoughts about this suggested change?
Since these globals are only available at runtime when the
--experimental-webstorageis passed, I'd say it's confusing to provide them without needing runtime checks.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The convention within @types/node (mostly) is not to type features that live behind flags as possibly
undefined.Not only does this force type checking/assertions on projects that know their own build environment, but for global variables, it's just incorrect: it's not the case that the variable is potentially undefined, it's that it potentially doesn't exist, and those are not equivalent.
The TypeScript compiler is perfectly happy here, which makes it a rather jarring experience when you run this and encounter
Uncaught ReferenceError: localStorage is not defined.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ophf 🤔 You're right. That case is not covered. I wondered if using
neverwould be better, but it doesn't seem to be the case.At the very least, this could have an experimental tag? Not that I'd expect great IDE support for highlighting that 😔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing to note is that you still need to end up in a position where loading the Node types and the DOM types together doesn't fall over; DT should be testing that already, and I would suspect that adding
| undefinedwould probably cause errors.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another alternative to keep things sane around types for experimental features would be to separate these experimental types into a separate file, such that users can opt into the feature via a simple top-level import:
Is that something which has been discussed already?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's a good idea to introduce imports which would crash at runtime; there's no way to enforce that an import is only used as a type import.
IMO this seems like a lot of work for not too much benefit; I'll note that the DOM types have some cases like this already for features that are only enabled in secure contexts, and those aren't even experimental.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree and that's the point of my initial comment. In its current form the
localStorageis declared while most likely not even defined, resulting in theUncaught ReferenceError: localStorage is not definedfor the average user, as @Renegade334 mentioned above. Putting this in a separate file, users would at least have to opt-into getting these experimental types declared on the global namespace.