Add definitions for WeakRef and FinalizationRegistry #38232

ikokostya · 2020-04-28T17:19:00Z

Fixes #32393

Link to proposal https://github.com/tc39/proposal-weakrefs/blob/master/reference.md

Fixes #32393

msftclas · 2020-04-28T17:19:13Z

All CLA requirements met.

…onal

orta · 2020-05-12T15:08:04Z

Great, this looks good to me:

the proposal is at stage 3, so it's good to be merged
the JSDoc annotations match the reference (nice work there)
It doesn't look like anything is missing

The file naming fits too, thanks 👍

orta · 2020-05-12T15:08:36Z

I'll give Nathan a chance to look over it, but I'm good to merge

sandersn

Questions about the type of target and unregisterToken:

Maybe they should be unknown or {} instead.
The proposal uses 'value' and 'object' fairly consistently, but doesn't define them. Does the ecmascript spec do so?

sandersn · 2020-05-12T15:23:05Z

src/lib/esnext.weakref.d.ts

+     * @param callback If given, the registry uses the given callback instead of the one it was
+     * created with.
+     */
+    cleanupSome(callback?: (heldValue: any) => void): void;


it's weird that heldValue: any but target: object and unregisterToken: object. I know the proposal uses the terms 'value' and 'object' but I don't think they exactly match the type system's definitions. In particular, object means "non-primitive". Can you provide null, undefined, "foo", false or 3 for target or unregisterToken?

target must be an object, i.e. non-primitive type.
Specification: https://tc39.es/proposal-weakrefs/#sec-weak-ref-target

If Type(target) is not Object, throw a TypeError exception.

V8 implementation behavior:

$ node --harmony-weak-refs Welcome to Node.js v14.0.0. Type ".help" for more information. > new WeakRef(null) Uncaught TypeError: WeakRef: target must be an object at new WeakRef (<anonymous>) > new WeakRef(undefined) Uncaught TypeError: WeakRef: target must be an object at new WeakRef (<anonymous>) > new WeakRef(1) Uncaught TypeError: WeakRef: target must be an object at new WeakRef (<anonymous>) > new WeakRef("str") Uncaught TypeError: WeakRef: target must be an object at new WeakRef (<anonymous>)

unregisterToken must be an object (non-primitive type), or undefined.

Specification: https://tc39.es/proposal-weakrefs/#sec-finalization-registry.prototype.register

If Type(unregisterToken) is not Object,
a. If unregisterToken is not undefined, throw a TypeError exception.
b. Set unregisterToken to empty.

V8 implementation behavior (please note that v8 in Node.js still uses old specification name FinalizationGroup instead of FinalizationRegistry):

$ node --harmony-weak-refs > var fg = new FinalizationGroup(() => {}) undefined > fg.register({}, null, null) Uncaught TypeError: unregisterToken ('null') must be an object at FinalizationGroup.register (<anonymous>) > fg.register({}, null, undefined) undefined > fg.register({}, null, 1) Uncaught TypeError: unregisterToken ('1') must be an object at FinalizationGroup.register (<anonymous>) > fg.register({}, null, 'a') Uncaught TypeError: unregisterToken ('a') must be an object at FinalizationGroup.register (<anonymous>) > fg.register({}, null, false) Uncaught TypeError: unregisterToken ('false') must be an object at FinalizationGroup.register (<anonymous>)

heldValue can have any type.

Specification: https://tc39.es/proposal-weakrefs/#sec-finalization-registry.prototype.register
V8 implementation behavior:

$ node --harmony-weak-refs > var fg = new FinalizationGroup(() => {}) undefined > fg.register({}, null) undefined > fg.register({}, false) undefined > fg.register({}, 1) undefined > fg.register({}, {}) undefined

I'd suggest omitting cleanupSome from .d.ts, or otherwise somehow mark it as dispreferred/not tab-complete it. It's "normative optional" in the specification, it's currently under some debate, and it's not going to be part of what V8/Chrome ships initially. See whatwg/html#5446 for context.

If it's "normative optional" in the specification, it can be marked as optional in .d.ts. Anyway this declaration is available only under --lib esnext compiler flag, which means that it isn't stable. If in final stage of the proposal this method will be removed, corresponding TypeScript declaration should reflect this change before move to stable es2020 lib.

If Chrome's not going to ship with it initially, let's make this optional to start so that people have to prove that it's available before using it. (Only when strict: true of course.)

Suggested change

cleanupSome(callback?: (heldValue: any) => void): void;

cleanupSome?(callback?: (heldValue: any) => void): void;

ikokostya · 2020-05-12T16:18:08Z

Actually this behavior is the same as in WeakMap, where key must be an object:

TypeScript/lib/lib.es2015.collection.d.ts

Line 45 in 0326534

interface WeakMap<K extends object, V> {

sandersn · 2020-05-20T22:03:04Z

src/lib/esnext.weakref.d.ts

+     * @param callback If given, the registry uses the given callback instead of the one it was
+     * created with.
+     */
+    cleanupSome(callback?: (heldValue: any) => void): void;


If Chrome's not going to ship with it initially, let's make this optional to start so that people have to prove that it's available before using it. (Only when strict: true of course.)

Suggested change

cleanupSome(callback?: (heldValue: any) => void): void;

cleanupSome?(callback?: (heldValue: any) => void): void;

Jamesernator · 2020-07-22T06:43:45Z

Is there any reason FinalizationRegistry isn't generic? I've used it a bit on the older version in Node (with --harmony-weakrefs) and I generally found the typechecking was helpful when I defined the types.

littledan · 2020-07-22T07:29:45Z

cleanupSome has been moved into a separate Stage 2 proposal, so I recommend omitting it from the typings here.

willstott101 · 2020-08-18T15:27:36Z

Is there any reason FinalizationRegistry isn't generic? I've used it a bit on the older version in Node (with --harmony-weakrefs) and I generally found the typechecking was helpful when I defined the types.

What would you include in the Generic? target, heldValue, unregisterToken or multiple?

I suppose target for typedocs so you know what it's intended to be holding references to, though I don't think this would help much with correctness.
heldValue to be type-safe between your callback and register calls.
unregisterToken to be type-safe for all register and unregister calls.

I see a lot more value in the latter two, but in many cases you might only use one or the other. It doesn't seem obvious to me what the Generic implementation would be.

Jamesernator · 2020-08-19T06:59:24Z

What would you include in the Generic? target, heldValue, unregisterToken or multiple?

I've generally defined it with all 3 as:

declare global {
     class FinalizationRegistry<
         TValue extends object=object,
         THeldValue=any, 
         TUnregisterToken extends object=never
     > {
        constructor(
           finalizationCallback: (heldValue: THeldValue) => void,
        );
        register(
            object: TValue,
            heldValue: THeldValue,
            unregisterToken?: TUnregisterToken,
        ): void;
        unregister(unregisterToken: TUnregisterToken): void;
    }
}

I do agree you'd often only use some, it's a bit of a shame there aren't named generics. There are hacky ways around it (like this) but this isn't an existing convention in the TS standard lib.

type FinalizationOptions = {
    Value: object,
    HeldValue: any,
    UnregisterToken: object,
}

type Get<T extends object, K, Else> = K extends keyof T ? T[K] : Else;

declare class FinalizationRegistry<T extends Partial<FinalizationOptions>> {
    constructor(
        finalizationCallback: (heldValue: Get<T, 'HeldValue', any>) => void,
    );
    register(
        value: Get<T, 'Value', object>,
        heldValue: Get<T, 'HeldValue', any>,
        unregisterToken?: Get<T, 'UnregisterToken', never>,
    ): void;
    unregister(
        unregisterToken: Get<T, 'UnregisterToken', never>,
    ): void;
}

const f = new FinalizationRegistry<{ HeldValue: number }>((i /* number */) => {
    console.log(i);
});

f.register({} /* object */, 3 /* number */);

ykahlon · 2020-10-05T16:31:30Z

src/lib/esnext.weakref.d.ts

+    deref(): T | undefined;
+}
+
+interface WeakRefConstructor {


Hi, any chance to expedite this change?

Yes, it got lost in my email, but I think it's good to go after I re-run the github CI.

Follow up to #38232, based on #33844

KilianKilmister · 2020-10-08T10:40:27Z

Funny coincidence, i just started using weak refs/Finalisation registry in a project yesterday and was little annoyed that it wasn't typed (the typescript nightly i had installed was a few days old). Perfect timing.

Two notes on FinalisationRegistry:
In FinalizationRegistry.prototype.register it looks like heldValue is optional (tested in node-v14.13.0). While i didn't find a anything specific calling it optional in the proposal, on MDN at least it is stated that undefined is an acceptable value for heldValue

As for Generics, this is what i used:

T extends object => so it can be integrated nicely with a collection
V = void => this way a type can be declared in the constructor argument and TypeScript knows that the second
parameter of FinalizationRegistry.prototype.register is only needed if it has a non-default type (if V defaults to
undefined, TypeScript will still want that second argument as a literal and throw a compiler error otherwise:
finalisationRegistry.register(someObject, undefined) )
Token extends object = object => defaulting to object since it is optional and needs a default since it's following a
another defaulting generic

declare class FinalizationRegistry<T extends object, V = void, Token extends object = object> {
  constructor (callback: (heldValue: V) => void)
  register (target: T, heldValue: V, token?: Token): void
  unregister (token: Token): void
}

usage:

const registry = new FinalizationRegistry<Record<string, string>>(() => console.log('some cleanup optimisation'))
// => `FinalizationRegistry<Record<string, string>, void, object>`

const registry = new FinalizationRegistry<readonly string[], number, readonly string[]>((heldItem: number) => console.log(`Array of ${heldItem} hasn't been unregistered`'))
const arr = [1,2,3,4] as const
registry.register(arr, arr.length, arr)

class Collection<T extends object, Held = void> {
  #registry: FinalizationRegistry<T, Held, this>
  constructor (callback: (heldValue: Held) => void, iterable?: Iterable<[T, Held]>) {
    this.#registry = new (FinalizationRegistry)(callback)
    for (const [item, held] of iterable ?? []) this.#registry.register(item, held, this)
  }

  register (item: T, heldValue: Held) { this.#registry.register(item, heldValue, this) }
  reset () { this.#registry.unregister(this) }
}

How @Jamesernator did the Token extends object = never is interesting, tho. migth me a good choice too, but for the other two i think this way is the best.

ExE-Boss · 2021-01-10T11:38:07Z

src/lib/esnext.weakref.d.ts

+     * Creates a WeakRef instance for the given target object.
+     * @param target The target object for the WeakRef instance.
+     */
+    new<T extends object>(target?: T): WeakRef<T>;


It’s invalid to call WeakRef with undefined:

Suggested change

new<T extends object>(target?: T): WeakRef<T>;

new<T extends object>(target: T): WeakRef<T>;

I’m fixing this in #42274.

ExE-Boss

FinalizationRegistry should be generic.

I’m fixing this in #42274.

ExE-Boss · 2021-01-10T11:40:29Z

src/lib/esnext.weakref.d.ts

+
+declare var WeakRef: WeakRefConstructor;
+
+interface FinalizationRegistry {


Suggested change

interface FinalizationRegistry {

interface FinalizationRegistry<T> {

ExE-Boss · 2021-01-10T11:40:39Z

src/lib/esnext.weakref.d.ts

+     * object. If provided (and not undefined), this must be an object. If not provided, the target
+     * cannot be unregistered.
+     */
+    register(target: object, heldValue: any, unregisterToken?: object): void;


Suggested change

register(target: object, heldValue: any, unregisterToken?: object): void;

register(target: object, heldValue: T, unregisterToken?: object): void;

ExE-Boss · 2021-01-10T11:40:55Z

src/lib/esnext.weakref.d.ts

+}
+
+interface FinalizationRegistryConstructor {
+    readonly prototype: FinalizationRegistry;


Suggested change

readonly prototype: FinalizationRegistry;

readonly prototype: FinalizationRegistry<any>;

ExE-Boss · 2021-01-10T11:41:12Z

src/lib/esnext.weakref.d.ts

+     * Creates a finalization registry with an associated cleanup callback
+     * @param cleanupCallback The callback to call after an object in the registry has been reclaimed.
+     */
+    new(cleanupCallback: (heldValue: any) => void): FinalizationRegistry;


Suggested change

new(cleanupCallback: (heldValue: any) => void): FinalizationRegistry;

new<T>(cleanupCallback: (heldValue: T) => void): FinalizationRegistry<T>;

Add definitions for WeakRef and FinalizationRegistry

0ae7ab6

Fixes #32393

Mark callback parameter in FinalizationRegistry#cleanupSome() as opti…

a5ce6c2

…onal

sandersn requested review from sandersn and orta May 12, 2020 14:36

sandersn assigned orta May 12, 2020

orta mentioned this pull request May 12, 2020

Feature/initial reference docs tc39/proposal-weakrefs#196

Merged

orta approved these changes May 12, 2020

View reviewed changes

sandersn requested changes May 12, 2020

View reviewed changes

ikokostya requested a review from sandersn May 12, 2020 16:12

sandersn added the For Backlog Bug PRs that fix a backlog bug label May 20, 2020

sandersn requested changes May 20, 2020

View reviewed changes

Make FinalizationRegistry.prototype.cleanupSome optional

6548776

ikokostya requested a review from sandersn May 20, 2020 23:07

miniak mentioned this pull request Jun 8, 2020

refactor: replace v8Util.createIDWeakMap() with native WeakRef in tests electron/electron#24017

Closed

7 tasks

Remove FinalizationRegistry.prototype.cleanupSome()

b6c29c7

ykahlon reviewed Oct 5, 2020

View reviewed changes

Merge branch 'master' into weakrefs-dts

3c0ed5f

sandersn approved these changes Oct 5, 2020

View reviewed changes

sandersn merged commit bd1d8e5 into microsoft:master Oct 6, 2020

sandersn added a commit that referenced this pull request Oct 7, 2020

Fix missed tasks to add esnext.weakref

9befc78

Follow up to #38232, based on #33844

sandersn mentioned this pull request Oct 7, 2020

Fix missed tasks to add esnext.weakref #40981

Merged

sandersn added a commit that referenced this pull request Oct 7, 2020

Fix missed tasks to add esnext.weakref (#40981)

fc783a9

Follow up to #38232, based on #33844

ExE-Boss reviewed Jan 10, 2021

View reviewed changes

ExE-Boss mentioned this pull request Jan 10, 2021

feat(lib/es2021): Add type parameter to FinalizationRegistry #42274

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add definitions for WeakRef and FinalizationRegistry #38232

Add definitions for WeakRef and FinalizationRegistry #38232

ikokostya commented Apr 28, 2020

msftclas commented Apr 28, 2020 •

edited

Loading

orta commented May 12, 2020

orta commented May 12, 2020

sandersn left a comment

sandersn May 12, 2020

ikokostya May 12, 2020 •

edited

Loading

littledan May 13, 2020

ikokostya May 13, 2020 •

edited

Loading

sandersn May 20, 2020

ikokostya commented May 12, 2020

sandersn May 20, 2020

Jamesernator commented Jul 22, 2020 •

edited

Loading

littledan commented Jul 22, 2020

willstott101 commented Aug 18, 2020

Jamesernator commented Aug 19, 2020

ykahlon Oct 5, 2020

sandersn Oct 5, 2020

KilianKilmister commented Oct 8, 2020 •

edited

Loading

ExE-Boss Jan 10, 2021 •

edited

Loading

ExE-Boss left a comment •

edited

Loading

ExE-Boss Jan 10, 2021

ExE-Boss Jan 10, 2021

ExE-Boss Jan 10, 2021

ExE-Boss Jan 10, 2021

	cleanupSome(callback?: (heldValue: any) => void): void;
	cleanupSome?(callback?: (heldValue: any) => void): void;

	new<T extends object>(target?: T): WeakRef<T>;
	new<T extends object>(target: T): WeakRef<T>;


		declare var WeakRef: WeakRefConstructor;

		interface FinalizationRegistry {

	interface FinalizationRegistry {
	interface FinalizationRegistry<T> {

	register(target: object, heldValue: any, unregisterToken?: object): void;
	register(target: object, heldValue: T, unregisterToken?: object): void;

	readonly prototype: FinalizationRegistry;
	readonly prototype: FinalizationRegistry<any>;

	new(cleanupCallback: (heldValue: any) => void): FinalizationRegistry;
	new<T>(cleanupCallback: (heldValue: T) => void): FinalizationRegistry<T>;

Add definitions for WeakRef and FinalizationRegistry #38232

Add definitions for WeakRef and FinalizationRegistry #38232

Conversation

ikokostya commented Apr 28, 2020

msftclas commented Apr 28, 2020 • edited Loading

orta commented May 12, 2020

orta commented May 12, 2020

sandersn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ikokostya May 12, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ikokostya May 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ikokostya commented May 12, 2020

Choose a reason for hiding this comment

Jamesernator commented Jul 22, 2020 • edited Loading

littledan commented Jul 22, 2020

willstott101 commented Aug 18, 2020

Jamesernator commented Aug 19, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KilianKilmister commented Oct 8, 2020 • edited Loading

ExE-Boss Jan 10, 2021 • edited Loading

Choose a reason for hiding this comment

ExE-Boss left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msftclas commented Apr 28, 2020 •

edited

Loading

ikokostya May 12, 2020 •

edited

Loading

ikokostya May 13, 2020 •

edited

Loading

Jamesernator commented Jul 22, 2020 •

edited

Loading

KilianKilmister commented Oct 8, 2020 •

edited

Loading

ExE-Boss Jan 10, 2021 •

edited

Loading

ExE-Boss left a comment •

edited

Loading