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

Doing without metadata #6

Closed
bakkot opened this issue Mar 28, 2022 · 9 comments
Closed

Doing without metadata #6

bakkot opened this issue Mar 28, 2022 · 9 comments

Comments

@bakkot
Copy link

bakkot commented Mar 28, 2022

This proposal advanced to stage 3 without metadata, with metadata a potential future addition. I want to lay out a possible way to get the benefits of metadata from the current proposal (without metadata), without any any additional features.

Basically, the idea is that field-level decorators can store whatever additional information they need in a closure, which a class-level decorator can later associate with the class in a class initializer. For example, looking at (a simplified version of) this case, one could imagine

import { registerGnomeObject } from 'gi://GObject';

function makeGObject() {
  let properties = [];
  done = false;
  return {
    register: (val, ctx) => {
      ctx.addInitializer(function(){
        done = true;
        registerGnomeObject(this, properties);
      });
    },
    property: info => (val, ctx) => {
      if (done) throw new Error('@property decorator called after @register');
      properties.push({ name: ctx.name, isStatic: ctx.isStatic, info });
    },
  };
}

which would allow almost exactly the use envisioned there:

let GObject = makeGObject();

@GObject.register
class MyClass extends GObject.Object {
    @GObject.property({
        type: Boolean,
        default: false,
        flags: GObject.ParamFlags.READWRITE | GObject.ParamFlags.CONSTRUCT,
    })
    flag = false;
}

The only difference, for a consumer, is the need to do GObject = makeGObject() before each use.

Am I correct in thinking that this would work? It seems to me to meet most of the use cases for metadata with very little additional overhead, and without any additional semantics.

This design also has the (IMO) significant advantage of being encapsulated by default, rather than exposing the metadata to everyone. The decorator can of course choose to add an integration point to the class (e.g. this.prototype[MY_SYMBOL] = function(){...}) if it explicitly wants, but it's not the default behavior.

@mhofman
Copy link
Member

mhofman commented Mar 28, 2022

I think I would prefer some kind of native support for this pattern. For the class case, it's not too intrusive to require this kind of closure to build a context, however if/when we add support for args decorators, then it gets more onerous. That's particularly the case for class/object methods and their args, where you'd have to create all the entangled method specific decorators ahead of the class/object definition.

I'm not sure what this context binding should look like. E.g. a method arg will probably need to use the method context to be coupled to a method level decorator, but maybe some cases also need to class/object context as well?

@bakkot
Copy link
Author

bakkot commented Mar 28, 2022

if/when we add support for args decorators, then it gets more onerous

If metadata is only worthwhile given some other future feature, I would expect metadata to come as part of that future proposal.

@mhofman
Copy link
Member

mhofman commented Mar 28, 2022

That's not what I meant. It seems like the concept of metadata is useful for class and its fields and methods. For that use case it's not too onerous to create an external context ahead of the class declaration. However I want to make sure we don't forget about future use cases like decorators for arguments for example, where creating an ad-hoc context is more problematic, especially when the context is really a nested one.

@bakkot
Copy link
Author

bakkot commented Mar 28, 2022

Ah, ok.

Well, to confess my biases here, I am (at present) very firmly against parameter decorators, so I am reluctant to let that potential future extension affect the design here very much.

@bakkot
Copy link
Author

bakkot commented Mar 28, 2022

To follow up on the OP, for the DI case (and similar, where you are storing metadata on instances), it seems like you can get by without a class-level decorator at all, by using the initializer function to add metadata:

const INJECTIONS = Symbol();

function inject(injectionKey) {
  return (_, context) => function(v) {
    if (!Object.hasOwn(this, INJECTIONS)) {
      this[INJECTIONS] = [];
    }
    this[INJECTIONS].push({ injectionKey, set: context.set });
    return v;
  };
}

class Container {
  registry = new Map();

  register(injectionKey, value) {
    this.registry.set(injectionKey, value);
  }

  lookup(injectionKey) {
    this.register.get(injectionKey);
  }

  create(Class) {
    let instance = new Class();

    for (let { injectionKey, set } of instance[INJECTIONS] ?? []) {
      set.call(instance, this.lookup(injectionKey))
    }

    return instance;
  }
}

class Store {}

class C {
  @inject('store') #store;

  get store() {
    return this.#store;
  }
}

let container = new Container();
let store = new Store();

container.register('store', store);

let c = container.create(C);

c.store === store; // true

Right?

(Edit: though I guess this incurs a small per-instantiation cost, which is maybe not worth it.)

@justinfagnani
Copy link

The only difference, for a consumer, is the need to do GObject = makeGObject() before each use.

That and the additional syntax needed at decoration sites make for pretty onerous requirements, IMO.

Right now in our library we have a @customElement() decorator that reads metadata written by class field @property() decorators. The DX would be pretty badly impacted without a way for them to communicate.

What we might do (besides skip standard decorators until metadata is ready) is to assume that there are no nested class definitions and have @property() write to a global list, and @customElement() read from it in and flush it in an initializer.

@bakkot
Copy link
Author

bakkot commented Mar 28, 2022

is to assume that there are no nested class definitions

You don't need to make that assumption; you can do a push when the class decorator is created and a pop when the initializer it created runs, as in

let register = () => {
  let old = globalPropertiesList;
  globalPropertiesList = [];
  return (val, ctx) => {
    ctx.addInitializer(function(){
      let properties = globalPropertiesList;
      globalPropertiesList = old;
      registerCustomElement(this, properties);
    });
  });
}


@register()
class A {
  // etc
}

This gets out of sync if a nested class throws during class definition; you can be a little defensive against that by having an actual stack and checking that height-at-pop matches height-at-push, though all you can do is give an error if it's not. It's kind of gross either way, which is why I suggested the paired-decorators approach.

@rbuckton
Copy link

To follow up on the OP, for the DI case (and similar, where you are storing metadata on instances), it seems like you can get by without a class-level decorator at all, by using the initializer function to add metadata:
[...]
Right?

(Edit: though I guess this incurs a small per-instantiation cost, which is maybe not worth it.)

That depends on the complexity of the DI system. Your example assumes a system with no circularities, and almost no up-front verification that the dependency graph can be satisfied before you begin allocation. This is especially important if one of your dependencies might have side effects (i.e., accessing an OS/file system resource, etc.).

For example, in https://github.com/rbuckton/service-composition I calculate up front whether the dependency graph can be satisfied before creating an instance. That is not viable with this approach.

As an alternative, I've proposed tc39/proposal-decorators#465, which would allow a non-static decorator to add a static extra initializer. This would provide a mechanism that would allow existing decorators to shoe-horn metadata via constructor/prototype access like they do today.

@pzuraq
Copy link
Collaborator

pzuraq commented Apr 24, 2022

Closing this issue as metadata has been opened up as a separate proposal here: https://github.com/tc39/proposal-decorator-metadata

The original idea for metadata has also been incorporated into the README.

@pzuraq pzuraq closed this as completed Apr 24, 2022
@ljharb ljharb transferred this issue from tc39/proposal-decorators Apr 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants