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

Add support for symbol-named class members #114

Open
kevinpschaaf opened this issue Jan 13, 2023 · 1 comment
Open

Add support for symbol-named class members #114

kevinpschaaf opened this issue Jan 13, 2023 · 1 comment

Comments

@kevinpschaaf
Copy link
Collaborator

The manifest cannot currently describe class members whose name is defined using a symbol, which is valid JS and not uncommonly used:

const foo = Symbol('foo');

class MyElement extends HTMLElement {
  [foo]() {
    console.log('foo called');
  }
}

This could be represented in the manifest by expanding the model for a class field's name to either be a string or Reference to a symbol's declaration. However, currently the name field for class members (currently defined on PropertyLike) is typed as a string and required, so a change to this type would represent a breaking change.

A (mostly?) backward compatible way to represent such class members could be to add an optional nameReference?: Reference field to ClassField and ClassMethod, which would be interpreted as a reference to a symbol's VariableDeclaration when present. Because name would still be required, it could be filled with a "display string" used for e.g. documentation viewers, e.g.:

{
  "name": "Symbol('foo')"
  "nameReference": {
    "package": "my-package",
    "module": "foo.js",
    "name": "foo"
  }
}

"Mostly" because tools could possibly be using name for purposes other than display, in which case it's still wrong, and might be better to just make a breaking change to support it.

@kevinpschaaf kevinpschaaf changed the title Allow for symbol-named class members Add support for symbol-named class members Jan 14, 2023
@justinfagnani
Copy link
Collaborator

References are still tricky... we need to give guidance here on whether this should point to the export of the module (yes) so that docs can resolve it to a public API member. It's the same with other references, just bears repeating, so that this works:

input:

const foo = Symbol('foo');

export class MyElement extends HTMLElement {
  [foo]() {
    console.log('foo called');
  }
}

export foo as bar;

manifest:

{
  "path": "foo.js",
  "exports": [
    {
      "kind": "js",
      "name": "bar",
      "declaration": {
        "name": "foo"
      }
    }
  ],
  "declarations": [
    {
      "kind": "class",
      "members": [
        {
          "kind": "method",
          "nameReference": {
            "package": "my-package",
            "module": "foo.js",
            "name": "bar"
          }
}

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