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

Metadata placement inconsistency with replaced classes #429

Closed
senocular opened this issue Aug 14, 2021 · 4 comments
Closed

Metadata placement inconsistency with replaced classes #429

senocular opened this issue Aug 14, 2021 · 4 comments

Comments

@senocular
Copy link
Contributor

Metadata on the prototype is assigned prior to replacement classes whereas static metadata is after. While this should mostly be OK due to metadata inheritance (depending on shadowing), it could cause problems for direct references to the original class, for example from those made within the class itself.

const META = Symbol('meta')
function setMeta (data) {
  return function (value, { setMetadata }) {
    setMetadata(META, data)
  }
}

function replace (C) {
  return class Replacer extends C {}
}

@replace
class C {

  @setMeta('static')
  static x = 1;
  
  @setMeta('instance')
  x = 2;
  
  reportMeta () {
    console.log(C[Symbol.metadata][META].public.x) // undefined <- Expected?
    console.log(C.prototype[Symbol.metadata][META].public.x) // "instance"
  }
}
  
console.log(C[Symbol.metadata][META].public.x) // "static" (C here is Replacer)
@pzuraq
Copy link
Collaborator

pzuraq commented Aug 14, 2021

C in both cases would refer to Replacer, all references to the original class are rebound to the replacement classes provided by any decorators.

@senocular
Copy link
Contributor Author

senocular commented Aug 15, 2021

Has this changed since PR2417? That still has class scope class name binding prior to replacement, so reportMeta's reference to C would be the original, not Replacer.

#329 covers this to an extent, but I wasn't sure what decisions (if any) came out of that.

@pzuraq
Copy link
Collaborator

pzuraq commented Aug 15, 2021

That was the intent of the design, if I made a mistake in the spec would definitely appreciate your help in pointing it out. I will be rereviewing and doing another round of refinement in the near future, but am currently focused on implementing the Babel plugin.

The outcome of #329 was that classes would continue to be applied before static class fields are applied. If decorators wish to run code after the class has been fully constructed and static class fields had been assigned, then they could use @init to add an initializer which would run at that time.

@senocular
Copy link
Contributor Author

Closing: intended design should not exhibit this behavior. Added a comment in the PR.

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

2 participants