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

feat: proposal for version 1.2 #8

Closed
wants to merge 7 commits into from

Conversation

daKmoR
Copy link

@daKmoR daKmoR commented Nov 10, 2019

  • Make it something for people to start with
  • Explain that it is versioned
  • Show example for version 1.2
  • Show changelog for version 1.2
  • Explain changes
  • Add Version History
  • Add existing tools

README.md Outdated Show resolved Hide resolved
- events
- methods
- [slots](https://html.spec.whatwg.org/multipage/scripting.html#the-slot-element)
- [CSS custom properties](https://drafts.csswg.org/css-variables/#defining-variables)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is "CSS custom variables" the more correct name?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A custom property is any property whose name starts with two dashes

I think the term is correct... I always say css variables and then correct myself to css custom properties as I think that is the spec name... but I could be totally wrong 🙈

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm being super picky, but you asked :) "Its primary goal is to offer...."
"Its" feels awkward here, given how short the doc is you're likely talking about custom-elements.json, but you could also mean this document maybe. I'd be more direct and say "The primary goal of custom-elements.json is to offer..."

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated
It is fully based on [Version 1](#version-1).
It adds all mentioned key points for web components.
It on purpose does not define a type system. This is left to the next iteration.
Therefore "type" should be seen as a type hint which can be presented to the end-user as documentation.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should put in some work to define the type field better

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added them via "Reference import" now 👍
please take a look 🤗

README.md Outdated Show resolved Hide resolved

Following is an example which shows all available capabilities.

```json
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think rather than an inline demo, we should build out the JSON Schema. I'd personally prefer to specify it in TypeScript, then use: https://github.com/YousefED/typescript-json-schema

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my experience having a demo json is the best way of "teaching" it to others. Having a json schema seems to even confuse people as often an assumption is made that their custom-elements.json has to be written in the same way. This may scare people off as a schema file looks kinda complicated.

And I can totally relate as for my learning type having a demo is the best way - for all other things I have to construct the demo in my mind first which usually comes with quite a cognitive load.

imho having a schema is an implementation detail and can be linked but should NOT be the main part of this page. Let's make the main readme more an explainer and have implementation details for a spec in a separate file.

Copy link

@runem runem Nov 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My suggestion here might be too web-component-analyzer-specific, but I'm currently working on an online playground that analyzes custom elements and shows the corresponding custom-elements.json output: https://runem.github.io/web-component-analyzer/?format=json (inspired by https://polymer.github.io/analyzer-playground/)

I think the playground provides a really good way to understand the custom-elements.json format, and therefore I think it would be a nice addition to this discussion :-)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@runem that is suupper awesome 🤗 I was really hoping we can get something like this at some point... but it's already there 💪

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added a link to the playground 👍

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"The main goal if this version is to document key API surfaces of web components."
Change "if" to "of"

@justinfagnani
Copy link
Collaborator

Thanks for getting this started!

I think we should probably start work on a branch, so it doesn't appear "published" to the repo quite yet, and also start with JSON schema so we have exact details on what the files mean.

README.md Show resolved Hide resolved
@daKmoR daKmoR changed the title feat: proposal for version 2.0 feat: proposal for version 1.2 Nov 11, 2019
@daKmoR
Copy link
Author

daKmoR commented Nov 11, 2019

update time

  • renamed it to version 1.2 (as vscode already has version 1.1)
  • added a schema.ts and schema.json
  • added path for tags (to get to the source file)
  • renamed cssProperties to cssCustomProperties
  • added privacy for properties and methods
  • added demos with name, description, url
  • added types per "Reference"

for example

      "properties": [
        {
          "name": "age",
          "type": {
            "name": "PersonAge",
            "import": "demo-wc-card/index.js"
          }
        }
      ],

so the type for age can be translated to import { PersonAge } from 'demo-wc-card/index.js.

in ts it is probably clearer

export interface PropertyDoc extends FieldDoc {
  // [...]
  /**
   * The type of the property
   *
   * If the type is built-in, then it is a string, e.g. string|number|...,
   * If the type is define in a module then reference to it.
   */
  type?: Reference | string;
}

/**
 * A reference to an export of a module.
 *
 * All references are required to be publicly accessible, so the canonical
 * representation of a reference it the export it's available from.
 */
export interface Reference {
  /**
   * Name of the type
   */
  name: string;

  /**
   * Bare module path to the file offering the named export
   * Can be translated to import { name } from import
   * @example
   * import { foo } from 'my-package/index.js';
   */
  import?: string;
}

Copy link

@bengfarrell bengfarrell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I could comment on the schema.json, but this felt weird too: "description": "Properties and modules can have a special privacy",
Did you mean special privacy enum? Maybe you meant for the reader to follow to the line below, but it felt weird to me.

- events
- methods
- [slots](https://html.spec.whatwg.org/multipage/scripting.html#the-slot-element)
- [CSS custom properties](https://drafts.csswg.org/css-variables/#defining-variables)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm being super picky, but you asked :) "Its primary goal is to offer...."
"Its" feels awkward here, given how short the doc is you're likely talking about custom-elements.json, but you could also mean this document maybe. I'd be more direct and say "The primary goal of custom-elements.json is to offer..."


Following is an example which shows all available capabilities.

```json

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"The main goal if this version is to document key API surfaces of web components."
Change "if" to "of"

## Linting
### Documentation Playground

With full information about the public api of a web component tools can provide UIs to trigger/execute these apis.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"public api of a web component tools can" - I think you need a comma to break this up: "public api of a web component, tools can..."


Linters should be able to produce warnings based on custom element defintions, such as warning if unknown elements are used in HTML templates.
If you are looking for a way to find out how your code will be represented in `custom-elements.json` you can visit the [playground of web-component-analyzer](https://runem.github.io/web-component-analyzer/?format=json). Paste you web component code in and see which data currently can get extracted automatically.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Paste your web component code" not "Paste you web component code"

/**
* Relative path to the js file which registers this web component
*/
path?: string;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we call it 'module' ? like in 'pkg.module'?
Just to avoid people shooting UMDs here...

@steel
Copy link

steel commented Nov 20, 2019

Not sure if this is the correct place to contribute but I've been getting custom-elements.json deployed with Storybook and wasn't happy with the current state of JSDoc generators for vanilla custom elements so I wrote my own. https://github.com/steel/custom-element-analyzer

It specifically uses JSDoc under the hood for all the parsing for maximum compatibilty. Hope it helps anyone one else who's using vanilla custom elements. Though it might work in other contexts as long as JSDoc can parse it.

@web-padawan
Copy link

@steel out of curiosity, why did you choose to write your own tool instead of using this one?
https://github.com/runem/web-component-analyzer

@steel
Copy link

steel commented Nov 20, 2019

@steel out of curiosity, why did you choose to write your own tool instead of using this one?
https://github.com/runem/web-component-analyzer

That one implements its own tree walker rather than using JSDoc to do the parsing for JSDoc blocks. I've been playing around with it for a few weeks and it's missing many JSDoc native features. I've also tried to submit patches without success. I'm using JSDoc in vanilla js custom elements exclusively so I wrote my own to tap into the JSDoc plugin infrastructure to handle the custom tagging.

runarberg pushed a commit to runarberg/stencil that referenced this pull request Apr 8, 2020
runarberg pushed a commit to runarberg/stencil that referenced this pull request Apr 11, 2020
runarberg pushed a commit to runarberg/stencil that referenced this pull request Apr 11, 2020
runarberg pushed a commit to runarberg/stencil that referenced this pull request Apr 14, 2020
runarberg pushed a commit to runarberg/stencil that referenced this pull request Apr 14, 2020
- Added tag slots with name, description
- Lists all [slots](https://html.spec.whatwg.org/multipage/scripting.html#the-slot-element) in the shadow dom like `<slot name="header"></slot>`
- The default slot is represented via an empty name `<slot></slot>`
- Added tag cssProperties with name, description, type

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This field is called cssCustomProperties in the schema and in the example above

@daKmoR
Copy link
Author

daKmoR commented Dec 16, 2020

superseded by the new format on master 👍

@daKmoR daKmoR closed this Dec 16, 2020
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

Successfully merging this pull request may close these issues.

None yet

9 participants