Skip to content

fix(lib/es2015): Fix definition of ProxyHandler #35594

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

Conversation

ExE-Boss
Copy link
Contributor

@ExE-Boss ExE-Boss commented Dec 9, 2019

This fixes several issues I found while comparing the definition of ProxyHandler to what’s actually defined in the specification.

Depends on:


Fixes #42894

@@ -1,18 +1,17 @@
interface ProxyHandler<T extends object> {
getPrototypeOf? (target: T): object | null;
setPrototypeOf? (target: T, v: any): boolean;
setPrototypeOf? (target: T, v: object | null): boolean;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Proxy exotic object’s [[SetPrototypeOf]] ( V ) internal method ensures that V is object | null before passing it to user code.

get? (target: T, p: string | symbol, receiver: any): any;
set? (target: T, p: string | symbol, value: any, receiver: any): boolean;
deleteProperty? (target: T, p: string | symbol): boolean;
defineProperty? (target: T, p: string | symbol, attributes: PropertyDescriptor): boolean;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Proxy exotic object doesn’t convert valid numeric indexes into numbers before passing them to user code, they remain as strings, which is how they’re actually implemented.

Choose a reason for hiding this comment

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

numbers never really exist as object keys, it's probably the biggest "lie" in typescript 🤷‍♀

set? (target: T, p: string | symbol, value: any, receiver: any): boolean;
deleteProperty? (target: T, p: string | symbol): boolean;
defineProperty? (target: T, p: string | symbol, attributes: PropertyDescriptor): boolean;
ownKeys? (target: T): ArrayLike<string | symbol>;
Copy link
Contributor Author

@ExE-Boss ExE-Boss Dec 9, 2019

Choose a reason for hiding this comment

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

If you return anything other that a string or symbol here, this will throw a TypeError at runtime (no, numbers don’t stringified here for whatever reason; see tc39/ecma262#1804 for discussion).

On the plus side, the spec doesn’t actually require that the returned value is an Array instance, just that it has a length property and doesn’t have holes.


Spec: 🔗

defineProperty? (target: T, p: string | symbol, attributes: PropertyDescriptor): boolean;
ownKeys? (target: T): ArrayLike<string | symbol>;
apply? (target: T & ((...args: any[]) => any), thisArg: any, argArray: any[]): any;
construct? (target: T & (new (...args: any[]) => object), argArray: any[], newTarget?: any): object;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These will only ever get called if the wrapped object is callable or constructable, respectively.

set? (target: T, p: PropertyKey, value: any, receiver: any): boolean;
deleteProperty? (target: T, p: PropertyKey): boolean;
defineProperty? (target: T, p: PropertyKey, attributes: PropertyDescriptor): boolean;
enumerate? (target: T): PropertyKey[];
Copy link
Contributor Author

@ExE-Boss ExE-Boss Dec 9, 2019

Choose a reason for hiding this comment

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

The enumerate(…) method was removed in ES2016 from both Reflect and ProxyHandlers, due to implementers deciding against it.

The type is wrong anyway, since it’s specified to return Iterator<any>.

@ExE-Boss ExE-Boss changed the title fix(lib/es2015): Fix definition of PromiseHandler fix(lib/es2015): Fix definition of ProxyHandler Dec 19, 2019
@sandersn sandersn added the For Backlog Bug PRs that fix a backlog bug label Feb 3, 2020
@sandersn sandersn requested review from sandersn, orta and rbuckton March 10, 2020 21:56
@sandersn sandersn self-assigned this Mar 10, 2020
@ExE-Boss ExE-Boss marked this pull request as draft December 16, 2020 02:59
@typescript-bot typescript-bot added For Uncommitted Bug PR for untriaged, rejected, closed or missing bug and removed For Backlog Bug PRs that fix a backlog bug labels Dec 16, 2020
@typescript-bot
Copy link
Collaborator

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

@ExE-Boss ExE-Boss force-pushed the lib/es2015/fix-proxy-handler-definition branch from a15bf40 to 64743c4 Compare January 4, 2021 10:52
@ExE-Boss ExE-Boss marked this pull request as ready for review January 4, 2021 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

ProxyHandler.apply method
5 participants