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

Reflect.getMetadata not working on proxified objects #106

Closed
nicomouss opened this issue Apr 14, 2019 · 8 comments
Closed

Reflect.getMetadata not working on proxified objects #106

nicomouss opened this issue Apr 14, 2019 · 8 comments

Comments

@nicomouss
Copy link

nicomouss commented Apr 14, 2019

See the right behaviour of Reflect.get() below, compared to Reflect.getMetadata() when a proxy is involved.

import "reflect-metadata";

const METADATA_KEY = Symbol();

const post = {
    id: 1000
}

Reflect.defineMetadata(METADATA_KEY, "Hello", post);

const postProxied = new Proxy(post, {});

console.log(`${Reflect.get(post, "id")}`); // OK, displays 1000
console.log(`${Reflect.get(postProxied, "id")}`); // OK, displays 1000

console.log(`${Reflect.getMetadata(METADATA_KEY, post)}`); // OK, displays "Hello"
console.log(`${Reflect.getMetadata(METADATA_KEY, postProxied)}`); // NOT OK, undefined metadata

My question then: how do I access an object's metadata when this object has been proxified?
Everything shouldn't be transparent like it is when using Reflect.get()?

@ljharb
Copy link

ljharb commented Apr 14, 2019

I’d think the Proxy has to allow you to do that, by having Proxy traps and a target argument to the Reflect method - otherwise you can’t/shouldn’t.

@nicomouss
Copy link
Author

nicomouss commented Apr 15, 2019

@ljharb Thanks for your feedback.

Maybe I am missing something here, but I guess everything should be transparent, and the one-to-one match between proxy methods traps and Reflect API should be kept consistent.

If in native Reflect API we have:

Reflect.apply()
Reflect.construct()
Reflect.defineProperty()
Reflect.deleteProperty()
Reflect.get()
Reflect.getOwnPropertyDescriptor()
Reflect.getPrototypeOf()
Reflect.has()
Reflect.isExtensible()
Reflect.ownKeys()
Reflect.preventExtensions()
Reflect.set()
Reflect.setPrototypeOf()

Then Proxy handler traps allowed are:

handler.apply()
handler.construct()
handler.defineProperty()
handler.deleteProperty()
handler.get()
handler.getOwnPropertyDescriptor()
handler.getPrototypeOf()
handler.has()
handler.isExtensible()
handler.ownKeys()
handler.preventExtensions()
handler.set()
handler.setPrototypeOf()

Now if reflect-metadata is extending the Reflect API with these methods:

Reflect.defineMetadata();
Reflect.hasMetadata();
Reflect.hasOwnMetadata();
Reflect.getMetadata();
Reflect.getOwnMetadata();
Reflect.getMetadataKeys();
Reflect.getOwnMetadataKeys();
Reflect.deleteMetadata();

Then it should also extend Proxy handler with the following trap methods:

handler.defineMetadata();
handler.hasMetadata();
handler.hasOwnMetadata();
handler.getMetadata();
handler.getOwnMetadata();
handler.getMetadataKeys();
handler.getOwnMetadataKeys();
handler.deleteMetadata();

Then things would be consistent.

As for now and current implementation of reflect-metadata: if at any point in a third-party library some code is proxyfying an object on which I have set up metadata beforehand, then I suddenly loose access to these metadatas ... Whereas the proxy should transparently forward the Reflect.getMetadata() call to the targeted object.

Things are confusing here because in the reflect-metada documentation (https://github.com/rbuckton/reflect-metadata - see Goals section), we have:

Metadata should be available not only on an object but also through a Proxy, with related traps.

Has something been implemented somewhere regarding that? Any example available?

@ljharb
Copy link

ljharb commented Apr 15, 2019

I agree with you; it’s been established many times in committee that nothing is allowed on Reflect that isn’t also a proxy trap.

@aigoncharov
Copy link

aigoncharov commented Jan 25, 2020

@nicomouss did you find a solution other than copying metadata manually?
@ljharb according to MDN if a trap has not been defined, the default behavior is to forward the operation to the target. That means that even we don't have Proxy traps for metadata, we still should be able to read it from the original object wrapped in a proxy.

@ljharb
Copy link

ljharb commented Jan 25, 2020

@aigoncharov yes, but if there isn’t a Proxy trap, there isn’t allowed to be a corresponding function on Reflect.

@aigoncharov
Copy link

aigoncharov commented Jan 25, 2020

@rbuckton here's the live demo based on the code provided by @nicomouss

@klerick
Copy link

klerick commented Nov 28, 2020

Do you have any news about this issue?

@rbuckton
Copy link
Owner

We're not planning on any further changes to how reflect-metadata works with respect to proxies. If we allowed metadata to pass through a proxy without the ability to intercept an operation in a proxy handler, there would be no way to prevent private implementation details in metadata from leaking through a proxy/membrane.

We're discussing how metadata should be handled in ECMAScript as part of the decorators proposal. Currently, we have two options under consideration:

  1. Adding a getMetadata proxy trap to allow interception or forwarding of the request, or
  2. Attaching metadata via a Symbol.metadata built-in symbol, which could then be intercepted via existing handlers.

The current implementation utilizes a WeakMap, and WeakMaps also do not automatically flow through proxies. The only approach I can recommend for now is to copy metadata from the target to the proxy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants