-
Notifications
You must be signed in to change notification settings - Fork 27
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
Initial schema #9
Conversation
what is a demo json for this? |
* The attributes that this element is known to understand. | ||
*/ | ||
attributes?: AttributeDoc[]; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
having no "flattened" properties here makes is severely harder to list them for documentation?
e.g. I need to go to the class get all members and mixins and construct it myself... quite cumbersome :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I would also prefer a flat version of custom-elements.json
. Moving the complexity to the "builder" of the file instead of the "consumer".
But maybe there is a use case that would change my mind.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can definitely do flattened properties. In the Polymer Analyzer we flatten, but in each member indicate where they originated from: https://github.com/Polymer/tools/blob/master/packages/analyzer/src/analysis-format/analysis-format.ts#L327
We can do the same here, though I think that should be expanded from a string to a reference type.
This allows us to show where members are inherited from in the documentation viewer UI. Examples here: https://polymer-library.polymer-project.org/3.0/api/polymer-element
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Best of both worlds, I like that
* The shadow dom content slots that this element accepts. | ||
*/ | ||
slots?: SlotDoc[]; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
misses css custom properties right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they are now exports of the class? seems kinda like a stretch as to have css variables as part of javascript class exports?
also seems same problem as with not having flattened properties... tough to work and imho also to understand 🙈
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add the styles section. This is in an interface that extends a ClassDoc, so ti has everything classes do plus custom element specifics.
overall this seems to move all the important parts to a full description of the javascript class... with inheritance and all. which opens a whole lot more questions? what happens if we extend/mixin from an external node_modules package? do we then add this info to the so single file analysis is then out of the picture 😭
Merging this will also hinder to move forward with the possibility to allow defining individual meta files per custom elements (think that every element in a repo has I really loved the simplicity of the format that this is my tag name and here are all my properties, attributes, ... and you don't need to know where they come from. (optionally we can add a class mixin/hint to help for the documentation but that's it) Overall I would not see this as a goal of
PS: this also leave is more open for other web-components libraries like haunted, stencil, ... |
Thanks Justin and Thomas to kick this off 👏 I'm intimidated with the details of class and mixins. I'm afraid of the adoption friction this could create. I would stay on the "outside shell" description of the custom-element. I would also favor a flat description that is easy for the "consumer"-side of My 2 cents |
BTW the authoring of the schema in ts is really nice - much easier to read than the output schema 👍 |
/** | ||
* A markdown description of the slot. | ||
*/ | ||
description?: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe there should be a property to define allowed content?
Im with George on this, from my point of view, im seeing types and custom-elements.json as a separate thing. For example, for the extension I'm working on: all I need to know for the 'tweak' functionality is what the attributes are, what the properties are. Then I need some type information like: str, bool, number, object, array. Thats all I need. I think we should aim for that, and then if people want to, they can hook up their typescript definitions file somehow that could give extended information (eg: its an object, but this is what the object looks like, or: its an array of Products or whatever) Imo custom-elements.json should not try to solve typing and I see it as a (not entirely, but somewhat) separate concern |
I think we need to consider the full set of use cases for this data. Documentation viewers, like webcomponents.org will need to show both the HTML-side and DOM/JS-side of element interfaces. Because elements are classes, this means showing properties and their types, methods and their parameter and return types, etc. As properties and methods accept complex object types, we need to document them too. Libraries like LitElement/lit-html will need to document arbitrary things like variables and functions. We had a lot of success with the Polymer Analyzer providing enough information for documentation viewers and using the same data for IDE integrations where type may also be provided by TypeScript or other analysis. I really don't think types are separate here. If TypeScript provided a standard JSON format for types, or if JSDoc had a standard JSON format for analysis result, maybe, but they don't. The serialization of type information into an easily consumable format, unlike .d.ts files, is a huge advantage to tools based on this format.
I don't think this will negatively affect adoption at all, but is more likely to increase it. There are only two ways that this file will be produced: by hand or with a tool. A tool is going to have no problem filling in the details allowable here, and a file written by hand can just omit almost everything. For example, this is a valid file under this schema: {
"schema-version": "2.0.0",
"modules": [{
"path": "",
"exports": [{
"kind": "class",
"name": "MyElement",
"tagname": "my-element",
"superclass": "HTMLElement"
}]
}]
} This defines a class with no additional properties on top of HTMLElement. Even if you used mixins to define properties, you could just document the properties and leave out the mixins. The point of having optional mixins field is to allow for documenting the mixins for use with documentation tools that want to do cross-references, etc. The presence of the option doesn't increase complexity for authors or tools that ignore it. |
Exactly, because this reflects the reality of web components. You cannot accurately document a web component without documenting the JavaScript class it's implemented with.
These questions exist whether the schema provides answers or not. What does happen when we extend from a class in another package? Without this information representable in the schema, the answer is that we can't even express this common situation. With space for this information we can, even if some tools don't produce it.
It already is in practicality. A tool that analyzes components built with LitElement really should be analyzing LitElement itself to copy down properties and methods from it. Not doing that will create a very incomplete picture of the element's API.
I don't see why this would be true. The format organizes APIs by module, you will be able to concatenate the module lists from multiple packages with no duplication. If you are analyzing multiple modules in the same package, you should absolutely be producing a single file. I don't think we've discussing having multiple
We really need to discuss this in use cases if this is something you want to pursue. How would tools find all these files? Right now a tool can just look for the presence of a single well-known file name. If we have multiple, open-ended filenames, tools will need to know how to find and collate them. That would definitely be an increase in compelxity.
That's totally supposed within this format.
There is no current other file format for the JavaScript side of things, and there is no clear line between elements and JavaScript. Even documenting properties already crosses over from pure HTML-side API docs and into JavaScript docs. As soon as you have a property that accepts a complex value, you need to document that value. If it's a function-typed value, you need to be able to document arbitrary functions. When you try to document components along with the libraries they're built-with, you need even more JS documentation abilities. We have a lot of experience trying to provide these tools for web components, and it's a basic truth of the problem that documenting web components is a superset of the problem of documenting JavaScript. Web components are just JavaScript objects with additional API surfaces. I think trying to separate things will lead to a much worse developer experience, where the documentation for a single element is spread across two formats and two viewers. Like one for attributes, and a JSdoc/TSdoc one for the JS API. That would be very unfortunate.
I don't see how allowing a more complete description of components would not allow for all possible web component implementation libraries. There's nothing about Haunted or Stencil that wouldn't work here. |
with I don't think Most design system I could find document only the important properties/events (if even) as giving all the info is rarely what the user needs...?
If we could provide the values used in the examples (usually name, description, type, default) we are already golden 🤗 I, however, see that we have different points of view on this matter - let's see what the other guys are saying. |
@daKmoR the first like I clicked on above was the Gestalt link. This is a React library, so it has a lot less to document as React components don't typically have methods, nonetheless, I see several problems that would be solved by fuller documentation. The SearchField component has property types that are complex JavaScript object: https://pinterest.github.io/gestalt/#/SearchField like Those are referenced by name only and no additional information is available about them. There are several data types that are documented inline, which is fine if they're simple but won't scale to references to more complex interfaces or shared classes. The first link in the list shows some problems too. https://design-system.pluralsight.com/components/button#proptypes It references some enum values in the prop types, but has to expand out each enum and just say in the text things like "visual style (from So even though these are React components and have a limited API surface, the docs could be improved with general JS/TS doc abilities. Again, tools don't have to use these fields or generate these fields. They're optional. A Storybook generator could ignore many things if it didn't have a place to put them. An analyzer for a specific library can not generate them. But for LitElement we definitely will want to produce analysis results for the full APIs as we have done for Polymer. So we'll definitely end up with some file format with these capabilities. I'd rather not have two formats. |
I have another idea 🤗 How about combining and linking it? Basic idea is to have both
Where tags would be a flat list of attributes, properties, methods, ... And modules would be the full module exports "tree" including mixins and separate properties/methods/... list for each "prototype". TagDocs would have a class which links to the correct module export it could look something like this (just psoido code 🙈 ) export interface PackageDoc {
version: number;
tags: TagDoc[];
modules: Array<ModuleDoc>;
}
export interface TagDoc {
name: string;
class?: ClassDoc,
attributes?: TagAttributeDoc[];
events?: TagEventDoc[];
slots?: TagSlotDoc[];
cssCustomProperties?: TagCssCustomPropertyDoc[];
cssShadowParts?: TagCssShadowPartDoc[];
properties?: TagPropertyDoc[];
methods?: TagMethodDoc[];
demos?: TagDemoDoc[];
}
export interface ModuleDoc {
path: string;
description?: string;
exports?: Array<ExportDoc>;
}
export type ExportDoc = ClassDoc|FunctionDoc|VariableDoc;
export interface ClassDoc {
kind: 'class';
name: string;
description?: string;
superclass?: Reference;
mixins?: Array<Reference>;
members?: Array<ClassMember>;
}
// ... |
Should we separate the concerns?
The first one can point to the types of the second one. Makes sense? |
I don't think there's as clear of a split as it seems, indicated by the fact that some people seem to want to put properties on the custom element side of things. Properties are part of the JS interface. This shows that the JS interface is important :) Also, Splitting the information across two files is strictly more complex than keeping them in a single file. I want to point out again that the JS-related fields are optional. If you don't want to generate them, don't. If you don't want to read them, don't. |
Custom elements are classes. So if we split their description, then tools will just need to recombine it. Let's say we have I think we can derive the appropriate schema by following the ground truth of the actual custom element definitions. Custom elements are defined in modules, modules are contain in packages. So a package needs to document it's modules, and the custom elements they contain. It's much easier to go from package>module>element and find all the elements with their complete interfaces, than the other way around. |
@justinfagnani But, I'm a little bit afraid of reinventing the wheel by not adopting typescript I gently disagree with the fact that it would be more complex to use But maybe there are other use cases of the class definition that I don't see today. |
The .d.ts files are still the best source for IDEs. In that case, only the additional custom elements information would likely be used, as the rest of the JS side would be derived from the .d.ts files or .ts files. Note that this is true of the property side of custom elements as well, even if there were no additional JS documentation. IDEs are only one use case for this file. One critical use case we have for the webcomponents.org catalog and custom design system catalogs is end-user documentation of the entire element interface. For this we need the JS docs, and we'd really like everything to be in an easily consumable JSON file, rather than having to spin up a TypeScript parser to get the JS docs, and then collate the information back together with the custom elements data. In the case of an analyzer like web-component-analyzer, it's already directly consuming TS/JS and is generating data based on the classes it finds. So the custom element / JS information is unified at the source. To split this information up into separate files, only to recombine it later is only more work and more dependencies to involve. |
ok I buy that 👍 Can we start by settling the tag part of the file: Attribs, props, slots, events, demos, cssprops? And then extend it with the class data in a second step? So much could be unlocked with just this first step if we can converge into something official. The catalog and an extension of open-wc, the web-component-analyzer and our support in webcomponents.dev are just the projects I know that could move forward if we have just that first step. But I state the obvious here I know :) |
@justinfagnani export interface TagDoc {
name: string;
class?: ClassDoc,
attributes?: TagAttributeDoc[];
events?: TagEventDoc[];
slots?: TagSlotDoc[];
cssCustomProperties?: TagCssCustomPropertyDoc[];
cssShadowParts?: TagCssShadowPartDoc[];
( properties?: TagPropertyDoc[]; )
( methods?: TagMethodDoc[]; )
demos?: TagDemoDoc[];
} This way we have the "HTML shell" that is available in a flatten format and then we have On the basis that the class definition stays manageable, we could have |
You might want open an issue in TSDoc for discussion. They already generated an AST from TSDoc, and I believe that can be easily converted to JSON. Re putting types into the JSON file: I don't think TypeScript would interoperate with it. I think the best way forward is to provide tools to convert custom-elements.json into multiple format, each for specific purposes. This way you can continue make changes to the schema while making features in different areas work as intended:
|
Hi all :-) I'm in the process of implementing this schema as an experimental output for web component analyzer so that we can explore it. You can follow my progress in this PR, and you can play around with it in the playground 🤗 During implementing the schema, I wrote down some questions/thoughts that I would like to add to this discussion. I see that there have already been some discussion around most of my points, and I could have been overlooking something, so please correct me where I'm wrong. 1. FlattenMy first question is about the flattening of members. There has been a lot of discussion around how to do it in this thread, but I'm not sure about what/if we landed on anything here. Therefore, the schema I implemented doesn't have flattened members. I'm looking for guidance/a discussion on this point. 2. Tag name vs class declarationI'm a bit unsure of what to output when the 3. Inherited custom element specificThe current schema has a required I want to raise this point, because an effect of (1) not including custom element specifics on the ClassDoc AND (2) not flattening, is that inherited custom element specifics are currently not included in the output for WCA. Only custom element specifics directly from the class defined as a custom element will be included. It could be solved by including (flattening) all inherited features on the CustomElementDoc in the output, but this wouldn't make it possible to easily lookup everything a MixinDoc contributes with. 4. Non-exported classes in the inheritance chainThis might be an edge case, but I'm including this question anyway. Only exported classes are included in the class A {}
// Only this class would be included in the exports array on ModuleDoc
export class B extends A {} Would we be fine with this behavior (eg. it's up to the author to export all classes in the chain)? |
Awesome news and awesome work @runem! Can't wait to try this for the element I was using my fork on. You raised some really good points. My initial thoughts: FlatteningI think flattening makes a lot of tools that consumer these files easier to build, especially when considering cross-package inheritance. As raised in the last meeting, this does have some issues around freshness, if a dependency has been updated, but I think more sophisticated tools that can consumer multiple packages can ignore inherited members if they have access to the originals. You point about anonymous classes might weigh in on this too. The easiest thing might be to say that they're flattened, but we don't have a reference to the class they inherited from. Tag name vs class declarationThis is a good point, especially when some people want to separate registration and class declaration into separate modules. I now think that tag names should be a distinct type of export for a module that reference their class declaration. Because there's a 1-1 relationship, the class should have a reference back to the name if there's a define() call. There are some possible edge cases where a class has multiple possible define calls, but I expect that to be extremely rare-to-non-existent. Inherited custom element specificAny class or mixin should be able to have custom element fields. We'll probably need some kind of field that says it has custom element data mixed in. Custom element fields should flatten if class members do. Non-exported classes in the inheritance chainI don't think this is going to be a completely rare edge case. It seems like there are only two options: 1) "export" an anonymous class, or 2) flatten the members into the nearest exported subclass. I think we could always flatten class members, but make a reference to where they were inherited from optional for anonymous superclasses. |
Thanks for your answer Justin! :-) In the newest version of the WCA playground, mixins and superclasses are included, and all features have been flattening. This required quite a bit of refactoring. You can see it in action here. However, I haven't yet added TagNameDoc to the output, and I don't yet include custom element specifics in non-custom-element-declarations, but this should be fairly easy to implement. TagNameDocIn order to add a TagNameDoc type to the schema I came up with a data structure of kind "tagName". What do you think of this suggestion (see example)? Or do you have a better suggestion? {
"version": "experimental",
"modules": [
{
"path": "definition.ts",
"exports": [
{
"kind": "tagName",
"name": "text-field",
"declaration": {
"name": "TextField",
"module": "declaration.ts"
}
}
]
},
{
"path": "declaration.ts",
"exports": [
{
"kind": "class",
"description": "A text field custom element",
"name": "TextField",
"attributes": [
{
"name": "size",
"defaultValue": "\"large\"",
"type": "\"small\"|\"large\""
}
]
}
]
} QuestionsHere are some more questions/considerations I wrote down while implementing the schema:
|
* TODO: restrictions on markdown/markup. ie, no headings, only inline | ||
* formatting? | ||
*/ | ||
summary?: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default?: string;
Is present in WCA output, but not in the schema
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
I just pushed a big commit where I cleaned up a lot of names and pulled out some shared interfaces in order to make things more consistent. Changes:
@runem this is a lot of changes here in names, but doesn't change the overall structure much outside of |
I would consider MyElement reachable from an export in this case. That guidance is there because we don't want packages documenting private declarations that are never reachable from outside their module. That's just noise. But there are often things that aren't directly exported that are still reachable and need to be documented. We can adjust phrasing on the comments. |
|
I see what you meant with Something else: Example: export interface Import {
kind: "named" | "default" | "import",
specifier: string,
identifiers: Array<Identifier>
}
export interface Identifier {
name: string,
alias?: string,
} import { LitElement } from 'lit-element';
import { css as style } from 'lit-element';
import html from 'lit-html';
import 'some-module';
class MyElement extends LitElement {
static styles = style``;
render() { return html`` }
}
// custom-elements.json
module: {
imports: [
{
kind: "named",
specifier: "lit-element",
identifiers: [
{
name: "Litelement"
},
{
name: "css",
alias: "style"
},
]
},
{
kind: "default",
specifier: "lit-html",
identifiers: [
{
name: "html"
}
]
},
{
kind: "import", // better name for this kind of import?
specifier: "some-module",
},
],
exports: [],
} Or would this only add clutter? Arguably you could also just pick a packages dependencies from its package.json. The danger with adding more stuff is that at some point we're just gonna become another AST implementation... |
I think ideally imports are an implementation detail. A package should be able to change around its dependencies and internal module structure without a public-facing change. I would want to start from the use case here. Presumably catalogs are going to want to label and filter components based on dependencies. You can do some of this via package.json, but if might be better to just allow some kind of labeling right in the schema and let tools filter that way. Github and npm tags are ok for this, though they're often stuffed with values to game search. Maybe we could do a key:value pair in a metadata field and tools can support well-known keys like |
Considering the following usecase: const NestedMixin = superclass => class NestedMixin extends NestedA(NestedB(superclass)) {} If a class uses the A solution for this could be to allow a import {NestedA, NestedB} from 'foo';
/**
* @mixin {NestedA}
* @mixin {NestedB}
*/
export const NestedMixins = superclass => class NestedMixins extends NestedA(NestedB(superclass)) {} import {NestedA, NestedB} from 'foo';
/**
* @mixin {NestedA}
* @mixin {NestedB}
*/
export function NestedMixins(superclass) {
return superclass => class NestedMixins extends NestedA(NestedB(superclass)) {}
} And add a export interface VariableDeclaration extends PropertyLike {
kind: 'variable';
mixins?: Array<Reference>
}
export interface FunctionDeclaration extends FunctionLike {
kind: 'function';
mixins?: Array<Reference>
} Real life usecase: |
@thepassle Polymer Analyzer handled this with three annotations: Polymer Analyzer looked at a pair of I'm trying to represent that here with a export interface MixinDeclaration extends ClassLike, FunctionLike {
kind: 'mixin';
} |
Wow, I had totally missed that 🤯 Sorry bout that. That should cover the usecase I mentioned 👍 |
chore: mixin is declaration
Inherit attrs events
@justinfagnani Suggestion on how to move forward:
I think having an example or demo repo/ What do you think of that plan? |
This is great to standardize and looking forward to building tools around what's developed here. If there's any data I can help provide is that this json file is generated by parsing all Ionic components: https://unpkg.com/browse/@ionic/[email protected]/dist/docs.json As you can see it has the same goals here, and I'm not proposing it uses this format, but rather it might be helpful to see what data is useful to generate Ionic's docs site. With this json file we're able to auto generate the each component's docs, such as: https://ionicframework.com/docs/api/button The button's API was actually pulled from the source: And we also found it useful to use the readme.md file for the free-form documentation: https://github.com/ionic-team/ionic-framework/blob/master/core/src/components/button/readme.md while the JSDocs in the components generated the actual API. The goal here is also similar to Capacitor's docgen for capacitor plugins: https://www.npmjs.com/package/@capacitor/docgen One thing, that I might be overlooking, is a string summarizing a method's signature, like |
Thanks for sharing @adamdbradley ! I'll go over it today and try to find the similarities and where the formats diverge, and what we can improve on. We really want to make sure
This is a fair point, we do have all the information available to construct the method signature under: |
Yes, thanks @adamdbradley for the links! I'll look at them too. Regarding method signature, I think I'd rather keep the data normalized and spin up helper libraries to stringify declarations. One reason is that we can offer a choice of to-string functions that can syntax highlight and hyperlink the signatures. I'd love it if it became expected practice in WC docs that they were cross-referenced internally, to other libraries, and MDN. Speaking of, I want to make sure we have optional references in all the schema types that need them for that to work. |
@thepassle I'm ok with merging this now and iterating. We have verbal +1s from the rest of the open-wc team, and you've been the most involved in the review here. I want to do another quick pass and set a pre-release shchema version so it's more clear that this isn't the final form. Then we can file issues and PRs for the rest. The only thing that would make me hesitate on actually publishing this is the name: #22 I'm just a little wary of naming the spec after the presumed filename. Thoughts (in that issue)? |
This could easily be implemented in https://github.com/open-wc/custom-elements-json/tree/master/packages/custom-elements-json-helpers 👍 (warning: very very unfinished, but now that eslint-plugin-lit-a11y is released, I'll be able to spend some more time on both the analyzer and helpers library)
I'll take a look |
Just took a closer look at your button example @adamdbradley As far as I can see, all the info you guys have in there is currently supported by The only thing I couldnt really figure out is this: I dont see that description in the source code, do you somehow add it later? 🤔 The only other thing I can think of thats different is indeed method signatures, but as discussed those could be easily implemented by helper libraries |
Thanks for the reviews everyone! Since it seems like there's consensus on this general shape of schema, I'm going to merge now and continue iteration in follow-on issues and PRs. |
The schema is authored in schema.ts and transformed into schema.json. Including both for review.