Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions types/node/globals.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Copy Markdown
Contributor

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-webstorage is passed, I'd say it's confusing to provide them without needing runtime checks.

Suggested change
var localStorage: Storage;
var localStorage: Storage | undefined;

Copy link
Copy Markdown
Contributor

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.

declare var localStorage: Storage | undefined;

// TypeScript tells me I should check that this isn't undefined...
if (localStorage) {
  localStorage.clear();
}

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.

Copy link
Copy Markdown
Contributor

@kraenhansen kraenhansen Jan 8, 2025

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 never would 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 😔

Copy link
Copy Markdown
Member

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 | undefined would probably cause errors.

Copy link
Copy Markdown
Contributor

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:

import type "node/experimental-webstorage";

Is that something which has been discussed already?

Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
Contributor

@kraenhansen kraenhansen Jan 9, 2025

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;

I agree and that's the point of my initial comment. In its current form the localStorage is declared while most likely not even defined, resulting in the Uncaught ReferenceError: localStorage is not defined for 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.


/**
* 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
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these use the DOM types if available?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Storage is interface, it extends existing DOM type if available.

// #endregion Storage

// #region Disposable
interface SymbolConstructor {
/**
Expand Down
4 changes: 2 additions & 2 deletions types/node/package.json
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
{
"private": true,
"name": "@types/node",
"version": "22.3.9999",
"version": "22.4.9999",
"nonNpm": "conflict",
"nonNpmDescription": "Node.js",
"projects": [
"https://nodejs.org/"
],
"tsconfigs": ["tsconfig.dom.json", "tsconfig.non-dom.json"],
"dependencies": {
"undici-types": "~6.18.2"
"undici-types": "~6.19.2"
},
"devDependencies": {
"@types/node": "workspace:."
Expand Down
52 changes: 52 additions & 0 deletions types/node/test/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,58 @@ access("file/that/does/not/exist", (err) => {
const result = util.parseArgs(config);
}

{
// args are passed `type: "boolean"` and allow negative options
const result = util.parseArgs({
args: ["--no-alpha"],
options: {
alpha: { type: "boolean" },
},
allowNegative: true,
});
// $ExpectType boolean | undefined
result.values.alpha; // false
}

{
// args are passed `default: "true"` and allow negative options
const result = util.parseArgs({
args: ["--no-alpha"],
options: {
alpha: { type: "boolean", default: true },
},
allowNegative: true,
});
// $ExpectType boolean | undefined
result.values.alpha; // false
}

{
// allow negative options and multiple as true
const result = util.parseArgs({
args: ["--no-alpha", "--alpha", "--no-alpha"],
options: {
alpha: { type: "boolean", multiple: true },
},
allowNegative: true,
});
// $ExpectType boolean[] | undefined
result.values.alpha; // [false, true, false]
}

{
// allow negative options and passed multiple arguments
const result = util.parseArgs({
args: ["--no-alpha", "--alpha"],
options: {
alpha: { type: "boolean" },
},
allowNegative: true,
});
// $ExpectType boolean | undefined
result.values.alpha; // true
}

{
const controller: AbortController = util.transferableAbortController();
const signal: AbortSignal = util.transferableAbortSignal(controller.signal);
Expand Down
3 changes: 3 additions & 0 deletions types/node/tls.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -843,6 +843,7 @@ declare module "tls" {
ciphers?: string | undefined;
/**
* Name of an OpenSSL engine which can provide the client certificate.
* @deprecated
*/
clientCertEngine?: string | undefined;
/**
Expand Down Expand Up @@ -885,12 +886,14 @@ declare module "tls" {
/**
* Name of an OpenSSL engine to get private key from. Should be used
* together with privateKeyIdentifier.
* @deprecated
*/
privateKeyEngine?: string | undefined;
/**
* Identifier of a private key managed by an OpenSSL engine. Should be
* used together with privateKeyEngine. Should not be set together with
* key, because both options define a private key in different ways.
* @deprecated
*/
privateKeyIdentifier?: string | undefined;
/**
Expand Down
6 changes: 6 additions & 0 deletions types/node/util.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1480,6 +1480,12 @@ declare module "util" {
* Whether this command accepts positional arguments.
*/
allowPositionals?: boolean | undefined;
/**
* If `true`, allows explicitly setting boolean options to `false` by prefixing the option name with `--no-`.
* @default false
* @since v22.4.0
*/
allowNegative?: boolean | undefined;
/**
* Return the parsed tokens. This is useful for extending the built-in behavior,
* from adding additional checks through to reprocessing the tokens in different ways.
Expand Down