[WIP] Implement new experimental JSON schema
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 :-)
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)
- [ ] All paths should be relative from root, but paths to non-project/external modules/packages/dependencies should just use the import specificer directly.
- [ ] Publish beta version of the CLI and update the playground
- [x] Use the summary JSDoc tag to fill out "summary"
- [x]
mixinson ClassDeclarations - [x]
parametersandreturnon ClassMethod - [ ]
staticon ClassField and ClassMethod - [ ] FunctionDeclarations and MixinDeclaration in the
declarationsarray on JavaScriptModule - [ ]
typeon Event - [ ]
demoson CustomElement
I 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.
- What is the current
schemaVersionfield onPackage? I assume that we are using semver versioning, so I just set it to0.0.1 - The fields
packageandmoduleare optional on the typeReference. 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? - The field
cssPropertiesonCustomElementis prefixed withcsswhilepartsisn't. Is this intended? - The type
Moduleisn't exported fromschema.tsright now (line 47) - 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. - Why is the
typefield onEventrequired? - Are mixins included twice the the
declarationsarray as bothMixinDeclarationand aFunctionDeclaration? -
MixinDeclarationextends bothClassLikeandFunctionLike. Does this mean that aMixinDeclarationincludes 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 fieldreturn.typeonMixinDeclarationneeds to reference aCustomElementtype 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:
-
I made a PR to export
Module -
I'll make a PR for parts
-
I think
0.0.0is 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. -
Something like
AnonymousClass1sounds 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 haveMixinDeclarationimplement 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
CustomElementsubclass 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