Open runem opened 4 years ago
Here is an update of what TODO's are still needed on my side:
There are still some things that have not been implemented yet, specifically:
TODO: (updated to reflect newest changes to schema.ts
)
mixins
on ClassDeclarationsparameters
and return
on ClassMethodstatic
on ClassField and ClassMethoddeclarations
array on JavaScriptModuletype
on Eventdemos
on CustomElementI will be focusing a bit more on lit-analyzer
the next couple of weeks until I have a 1.2.0
release, so I'm not sure how much progress I will be able to make on web-component-analyzer
during the next week or two :-)
Introduce a type to the exports array to be used when a custom element is defined in a given module and remove tagName from CustomElementDoc. For example:
I think the CustomElementDefinitionDoc
is a good idea, but I think by default a classdoc shouldnt be dependant on a tagname, right?
I implemented most of what's in the updated schema. While implementing the changes, I wrote down a couple of questions that I would like to discuss.
schemaVersion
field on Package
? I assume that we are using semver versioning, so I just set it to 0.0.1
package
and module
are optional on the type Reference
. What does it mean if these are not included? Does it make the reference global (such as "HTMLElement"), or does it mean that it references the same module as the module of the symbol that referenced it? Or both?cssProperties
on CustomElement
is prefixed with css
while parts
isn't. Is this intended?Module
isn't exported from schema.ts
right now (line 47)customElements.define("my-element", class extends HTMLElement { })
or a mixin returning an unnamed class? The declaration doesn't have a name, so it becomes impossible to reference and include it in the output.type
field on Event
required?declarations
array as both MixinDeclaration
and a FunctionDeclaration
?MixinDeclaration
extends both ClassLike
and FunctionLike
. Does this mean that a MixinDeclaration
includes the fields on the class returned by the mixin? If so, this structure wouldn't be able to contribute with custom-element-specific fields such as attributes. If this is by design, is it the idea that the field return.type
on MixinDeclaration
needs to reference a CustomElement
type declaration instead?Right now, WCA doesn't properly include mixins in the output because it must be extended to analyze and understand mixins in another way. Mixins are only analyzed while traversing inheritance starting from custom element class declarations, but should be extended to analyze all mixins tagged with the @mixin
JSDoc tag.
What is the current schemaVersion field on Package? I assume that we are using semver versioning, so I just set it to 0.0.1
I think it should still be 'experimental'
, but we should really be aiming for 1.0.0 soon
The fields package and module are optional on the type Reference. What does it mean if these are not included? Does it make the reference global (such as "HTMLElement"), or does it mean that it references the same module as the module of the symbol that referenced it? Or both?
If the type is local in the current module, then the module
field should point to the current module.
The field cssProperties on CustomElement is prefixed with css while parts isn't. Is this intended?
I think we should align in this case and name it cssPart
, the type is also CssPart
. Can you make a PR?
The type Module isn't exported from schema.ts right now (line 47)
Can you make a PR
How do I include unnamed classes in the output such as customElements.define("my-element", class extends HTMLElement { }) or a mixin returning an unnamed class? The declaration doesn't have a name, so it becomes impossible to reference and include it in the output.
Perhaps we should use a convention like AnonymousClass1
, AnonymousClass2
, etc for this specific case?
Why is the type field on Event required?
Because events can have types
Are mixins included twice the the declarations array as both MixinDeclaration and a FunctionDeclaration?
If its a mixin it should be a MixinDeclaration.
MixinDeclaration extends both ClassLike and FunctionLike. Does this mean that a MixinDeclaration includes the fields on the class returned by the mixin? If so, this structure wouldn't be able to contribute with custom-element-specific fields such as attributes. If this is by design, is it the idea that the field return.type on MixinDeclaration needs to reference a CustomElement type declaration instead?
Im not sure what you mean here, can you elaborate? The Mixins fields should be applied to the class that uses the Mixin, right? Not the other way around
Thanks for starting the implementation @runem!
@thepassle's answers are great here. A few things:
Module
0.0.0
is probably good as a version until we make sure there's minimal usability, and then go to 0.0.1 right away. Probably after we fix a few things here.AnonymousClass1
sounds good, but I think we should probably prefix it with a $
to make it look weird. We should give guidance on this in the schema docs so it's more likely to be consistent.I see the point about mixins. In Polymer Analyzer we had modeled mixins as if they were a first-class JS construct, and I'm a bit wary of this approach, so I was trying something different and definitely missed being able to mix-in custom element features. Coincidentally, what we theoretically want is for 'CustomElement' to be a mixin that can be applied to ClassDeclaration
. Since this is a custom-elements specific format, I think we can safely just have MixinDeclaration
implement the custom element fields. I'll make a PR for that.
As you hint at, another way to model mixins would be simply as functions that return a ClassDeclaration as a type. The problem I see there is that so far the types in this file are purely textual, with an allowance for spans being references. To have a function that returns a CustomElement
subclass be specially recognized as a mixin, we'd have to specify some syntax/semantics for types.
Another approach would be to have Mixin extend Function, but with one additional field that points to a declaration that's mixed in. That field could be a reference, so we don't care about the type syntax. We'd have the anonymous class issue here quite often.
I'm a little unsure here, so I would love implementation feedback too, from both analyzers and documentation viewers. I think this is the biggest thing to finalize before the format is stable. I'll start with a PR for making Mixin inherit from CustomElement, and we can go from there.
@runem I just made a PR for mixins that includes a comment on how to structure them: https://github.com/webcomponents/custom-elements-manifest/pull/31
This PR implements the proposed JSON schema found here as a new experimental output format called
json2
.I will have to make some more changes to the internal data structure in order to better output this schema.
You can test it at the playground here: https://runem.github.io/web-component-analyzer/?format=json2. I will make sure to update the playground when I finish more parts of the schema.
You can also clone this repo, build it and run
./cli.js ./dev/src/custom-element/custom-element.ts --format json2
. I might publish as beta version on NPM soon.While implementing this I wrote down some questions and considerations which I will add to the discussion at https://github.com/webcomponents/custom-elements-json/pull/9 during this week :-)