Skip to content
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

Add definitions for WeakRef and FinalizationRegistry #38232

Merged
merged 5 commits into from
Oct 6, 2020
Merged
Show file tree
Hide file tree
Changes from 2 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
1 change: 1 addition & 0 deletions src/lib/esnext.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
/// <reference lib="esnext.intl" />
/// <reference lib="esnext.string" />
/// <reference lib="esnext.promise" />
/// <reference lib="esnext.weakref" />
67 changes: 67 additions & 0 deletions src/lib/esnext.weakref.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
interface WeakRef<T extends object> {
readonly [Symbol.toStringTag]: "WeakRef";

/**
* Returns the WeakRef instance's target object, or undefined if the target object has been
* reclaimed.
*/
deref(): T | undefined;
}

interface WeakRefConstructor {
Copy link

Choose a reason for hiding this comment

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

Hi, any chance to expedite this change?

Copy link
Member

Choose a reason for hiding this comment

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

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

readonly prototype: WeakRef<any>;

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

@ExE-Boss ExE-Boss Jan 10, 2021

Choose a reason for hiding this comment

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

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.

}

declare var WeakRef: WeakRefConstructor;

interface FinalizationRegistry {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
interface FinalizationRegistry {
interface FinalizationRegistry<T> {

readonly [Symbol.toStringTag]: "FinalizationRegistry";

/**
* Registers an object with the registry.
* @param target The target object to register.
* @param heldValue The value to pass to the finalizer for this object. This cannot be the
* target object.
* @param unregisterToken The token to pass to the unregister method to unregister the target
* 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;
Copy link
Contributor

Choose a reason for hiding this comment

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

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


/**
* Unregisters an object from the registry.
* @param unregisterToken The token that was used as the unregisterToken argument when calling
* register to register the target object.
*/
unregister(unregisterToken: object): void;

/**
* Triggers callbacks for an implementation-chosen number of objects in the registry that have
* been reclaimed but whose callbacks have not yet been called.
*
* Note: The number of entries for reclaimed objects that are cleaned up from the registry
* (calling the cleanup callbacks) is implementation-defined. An implementation might remove
* just one eligible entry, or all eligible entries, or somewhere in between.
* @param callback If given, the registry uses the given callback instead of the one it was
* created with.
*/
cleanupSome(callback?: (heldValue: any) => void): void;
Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

@ikokostya ikokostya May 12, 2020

Choose a reason for hiding this comment

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

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

    1. 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>)
    
  2. unregisterToken must be an object (non-primitive type), or undefined.

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

    1. 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>)
    
  3. 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
    

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@ikokostya ikokostya May 13, 2020

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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;

}

interface FinalizationRegistryConstructor {
readonly prototype: FinalizationRegistry;
Copy link
Contributor

Choose a reason for hiding this comment

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

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


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

Choose a reason for hiding this comment

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

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

}

declare var FinalizationRegistry: FinalizationRegistryConstructor;