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

Docs: web components API table has wrong default value #9642

Closed
telpalbrox opened this issue Jan 27, 2020 · 8 comments
Closed

Docs: web components API table has wrong default value #9642

telpalbrox opened this issue Jan 27, 2020 · 8 comments

Comments

@telpalbrox
Copy link
Contributor

Docs: web components API table has wrong default value
When using @storybook/addon-docs the prop table is not displaying the default value generated from custom-elements.json with web-component-analyzer@1.02.

To Reproduce

  1. Create new web component storybook project
  2. Create a web component that has a default value in one property
  3. Configure and add @storybook/addon-docs
  4. Generate a custom-elements.json file using web-component-analyzer@1.02
  5. Go to the doc page of the web component
  6. See that "DEFAULT" column of the prop table is empty even when in custom-elements.json there is a default value.

Expected behavior
"DEFAULT" column of the prop table has the value specified in custom-elements.json.

Screenshots
Screenshot 2020-01-27 at 09 26 54

Code snippets
Component:

import { LitElement, html, property, customElement } from "lit-element";

@customElement("test-awesome")
export class AwesomeComponent extends LitElement {
    @property({ type: String }) name = "Awesome";

    render() {
        return html`${this.name}`;
    }
}

Additional context
After taking a quick look to the code in addons/docs/src/frameworks/web-components/config.js#L14 I can see that is looking for defaultValue but now it is called default in custom-elements.json. You can see an example of the new format here https://runem.github.io/web-component-analyzer/?format=json .
I guess as the format is experimental and it changed at some point 😓

Please, ping me if you need any more information.
I would be more than willing to provide a pull request to fix this problem 😄

@shilman
Copy link
Member

shilman commented Jan 27, 2020

cc @daKmoR

@daKmoR
Copy link
Contributor

daKmoR commented Jan 27, 2020

jup that is the spot 👍

yes it is not that stable yet 🙈

we could do default != undefined ? default : defaultValue to support both 🤗

@shilman
Copy link
Member

shilman commented Jan 28, 2020

@daKmoR Your call -- you have the most context on what's going on in the WC community. Since we're on 6.0 now we can make breaking changes, and we should do that where we can. When in doubt move things forward. In subsequent releases we'll need to be careful about this kind of stuff as the spec changes, and support back compat until 7.0.

@telpalbrox
Copy link
Contributor Author

I created a pull request with the suggested fix from @daKmoR. I kept compatibility with defaultValue as maybe it makes sense to include this change for a future 5.X release (if there is any).

If no more 5.X releases are planned then it makes sense to just make the incompatible change and rely only in default.

@shilman
Copy link
Member

shilman commented Jan 30, 2020

Gadzooks!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.0.0-alpha.2 containing PR #9655 that references this issue. Upgrade today to try it out!

You can find this prerelease on the @next NPM tag.

Closing this issue. Please re-open if you think there's still more to do.

@shilman shilman closed this as completed Jan 30, 2020
@telpalbrox
Copy link
Contributor Author

I updated to 6.0.0-alpha.2 and it works as expected now! Thank you very much 😃

@shilman
Copy link
Member

shilman commented Jan 30, 2020

Thanks @telpalbrox! Will patch this into 5.3 in a few days too

@shilman
Copy link
Member

shilman commented Feb 2, 2020

Yippee!! I just released https://github.com/storybookjs/storybook/releases/tag/v5.3.10 containing PR #9655 that references this issue. Upgrade today to try it out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants