Why add metadata to the class declaration

[Lodin]

I'm not sure I fully understand why metadata is added to the class declaration via some well-known symbol? I see here some disadvantages:

• It is unsafe. Any data we store as metadata can be easily found by any injected code that has access to the class declaration itself (e.g., can import it).
• Since it is unsafe, the proposal does not provide the same flow for private fields. Their decorators just don't receive context object. This may be confusing for the end user.

However, we already have context object that can serve us well here. We can use it in the following way:

• Member decorators only add metadata; class decorators only read it. The reason is the execution order. We cannot predict when and how the metadata is accessible within the property/method decorators; class decorators, on the other hand, are executed after all the member decorators. I also don't see any reason for class decorators to add the metadata because they usually don't work in a chain and hence don't need metadata shared.

For sure, there is still a way to steal some data through a malicious decorator but at least it can be caught and fixed by the developer. To make storage fully private, we can use the closure to produce connected decorators and WeakMap/WeakSet as the internal storage.

const visibility = {
  init(klass, context) {
    const {fields} = context.metadata.instance;

    for (const fieldName in fields) {
       if (fieldName.startsWith('#') && fields[fieldName].type === visibility.key) {
          Object.define(klass.prototype, fieldName.slice(1), {
            get: fields[fieldName].get,
          });     
       }
    }
  },
  key: Symbol('visible'),
  visible(accessors, context) {
    context.metadata = {
      type: visibility.key,
    };
    
    return accessors;
  }
}

@visibility.init
class Foo {
  @visibility.visible #bar = someSecretData;
}

[pzuraq]

I think it's important for code other than class decorators to be able to access metadata. For instance, it could be very useful for dependency injection, where the container is trying to read the metadata to understand what it should provide to the class.

class MyComponent {
  constructor(@service('foo') foo) {
    // ...
  }
}

The current proposal would allow this in general, though not for private fields/methods

[Lodin]

Sorry, I've never used parameter decorators and don't imagine correctly how they work. So let's say that in my post I don't touch them and speak only for class/member decorators.

[pzuraq]

Sure, metadata could still be useful for a DI system that assigns to class instance properties though:

class MyComponent {
  @service('foo') foo;
}

Ember.js's does something like this, but lazily via the getter, which is why I used a parameter decorator in the original example. The point being, external code other than class decorators may need access to the metadata. I think the case is most compelling for parameter decorators, but it may make sense for others as well.

[Lodin]

Hmm, could you please elaborate on this example a bit more? I'm not sure I caught your thought.

[pzuraq]

In dependency injection based systems, the container is responsible for instantiating classes and providing them with any instances of other classes that they may need. Usually, this is accomplished with a type system, but in JS we don't have that option, so many frameworks use decorators to tell the container what the value should be.

In Ember.js we do this lazily, so the first time you access the service it will look it up based on the name, and it doesn't need to expose metadata. But other frameworks, like Angular, do this eagerly, and as such the container would probably want to read the metadata as it was creating the class, to figure out what to inject.

This isn't the only instance of this type of usage of metadata we found. You can check out the ecosystem audit that was done, there are a few libraries that use metadata, such as Nest.js, and expect that metadata to be available to other classes.

[mhofman]

@Lodin I actually suggested a separate "placeholder" concept for private decorator only data: #328

[Lodin]

@pzuraq, I agree that providing metadata to any other code is an important feature. Though, I think that making private information public is easier than making public information private. E.g., if we have any private metadata stored in the context object, we could easily share it inside the class decorator, like:

const metadata = Symbol('metadata');

function shareMetadata(klass, context) {
  klass[metadata] = context.metadata;
}

@shareMetadata
class Foo {}

const metadata = Foo[metadata];

However, if you want to make public information private, that may be tricky.

function applyMetadata(klass, context) {
  const metadata = klass[Symbol.metadata];

  // work with metadata

  delete klass[Symbol.metadata];

 // or

  klass[Symbol.metadata] = undefined;
}

It has some disadvantages:

• delete may cause performance penalty.
• Removing metadata may be forbidden (well, that depends on the proposal).
• This operation is not mandatory and may be omitted by the decorator developer even if it is better to clean up the metadata when it is used within decorators.

So I think it is always preferable to make information private and when it is necessary, we can always share it. And not only adding a symbolic property to the class declaration but also using WeakMap and other structures.

Also one more argument for my suggestion: it is simpler. We reduce an amount of entities the end user should remember by combining Symbol.metadata with the context object.

[pzuraq]

Another possibility here, if privacy is a chief concern, would be to instead expose metadata via a query API such as the Reflect.metadata APIs. Then, to access metadata, you would have to do something like:

let metadataValue = Reflect.getMetadata(metadataKey, obj);

If the key can be any value, and not just a string, you could have private metadata by defining a Symbol or object in a module which is never exported. I think the context.metadata API would also need to be updated, but using the original example it could look like:

const VISIBLE = Symbol('visible');

export function init(klass) {
  let visibleFields = Reflect.getMetadata(VISIBLE, klass);

  for (const fieldName in visibleFields) {
    // Going to assume that `fieldName` here is the spelling of the field
    Object.define(klass.prototype, fieldName.slice(1), {
      get: visibleFields[fieldName],
    });
  }
}

export function visible(accessors, context) {
  context.setMetadata(VISIBLE, accessors.get);

  return accessors;
}

Now I think there isn't a way for anyone to access the VISIBLE metadata externally, since they don't have access to the symbol.

[pzuraq]

Also discussed this a bit further with @littledan, and this use case actually wouldn't be solvable with the current proposal. Metadata currently isn't allowed/exposed for private fields or methods, they idea being that there shouldn't be a way to introspect on private implementation details at all. The README outlines a separate way to gain access, which can be used for either collaborating or testing code, but would not really make this @visible use case anymore ergonomic.

I do still think that using a query based API to access metadata may be a good idea though, since it would allow other decorators to hide their implementation details.

[littledan]

I can definitely see the concern that you don't want to grant the whole world the ability to mutate your class metadata just because they import your class. Here's a sketch of an idea:

1. Context objects are frozen and reused across multiple calls to the decorator
2. That context object can be used as keys in WeakMaps to store metadata
3. Then, once the class is created, there's an API to look up the context object corresponding to a particular field or method.

Some issues with this idea:

• It seems weird that you can still look up the context object even if no one actually used it for metadata. Does this mean we have to create these objects even for un-decorated class elements?
• It's not clear what the API should be to get the appropriate context object from the class
• We'd still need a library to query metadata, given Metadata decorator Inheritability behaviour? #343

[bergus]

I would definitely recommend making metadata public, and solve the mutability issue by making allowing to make properties non-configurable if the decorator (or something else) chooses to do so.

[pzuraq]

Solutions to this issue are discussed in Hiding Metadata, which shows how you can prevent your metadata from being read by anyone other than yourself. One of the major advantages to using a well known symbol is that it allows all of the existing Reflect APIs to work, preventing us from having to increase the scope of this proposal dramatically. As such I'm going to close this issue.

[ljharb]

I'm confused how the symbol being well-known makes a difference for Reflect APIs?

[pzuraq]

The alternative would be associating metadata via a different API, such as Reflect.metadata. This would require us to design new reflection APIs for intercepting and modifying metadata (e.g. Proxy handler APIs and such)

[ljharb]

Things can only ever go on Reflect if they're Proxy traps, and "metadata" doesn't make sense as a trap there, so it wouldn't ever go on Reflect, whether the symbol was well-known or not.

Maybe I'm overindexing on the "well-known" part - are you saying that going with a symbol avoids complexifying existing APIs, as opposed to a side channel or internal slot?

[pzuraq]

Yes, that’s what I’m saying.