Web-components: Dynamic source snippets

[bennypowers]

Issue: #11588

Adds dynamic source to the web-components 'framework' package.

What I did

Emit the rendered story html (minus lit-html's placeholder comments) to the source channel.

Questions:

• Can we assume the story is always a lit-html TemplateResult? If not then what?
• How do i safely get a reference to the document?

How to test

• Is this testable with Jest or Chromatic screenshots? yes
• Does this need a new example in the kitchen sink apps? maybe?
• Does this need an update to the documentation? probably

👉 long time lurker first time poster. submitting this single-file PR for discussion and to arouse feedback from our good maintainers.

[nx-cloud]

Nx Cloud Report

CI ran the following commands for commit 9a5dccd. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

Status	Command
	nx run-many --target=prepare --all --parallel --max-parallel=15

---

Sent with 💌 from NxCloud.

[Westbrook]

In https://github.com/storybookjs/storybook/blob/next/app/web-components/src/client/preview/render.ts#L24 you can see that the web-components build accepts a few other story html shapes: TemplateResult, String, and Node. Would this need to be taken into account, or is the renderer HTML here a little less effected by the real source?

[bennypowers]

I dunno, might have to 🦆 type the return value of storyFn. I'd like to get some tests on this.

edit: Then again, as long as the return value is a renderable, we're golden, right?

[Westbrook]

Question re: decorators for the Storybook team...

Is is possible to resolve the root/origin story, so that the source here comes from that no matter the order in which the decorator is applied? Right now, it has to go first, or it will render the decorator code only, which isn't too bad, but would be nice not to require.

[bennypowers]

Tried to test this locally but got

 FAIL  addons/docs/src/frameworks/web-components/sourceDecorator.test.ts
  ● Test suite failed to run

    Jest encountered an unexpected token

    This usually means that you are trying to import a file which Jest cannot parse, e.g. it's not plain JavaScript.

    By default, if Jest sees a Babel config, it will use that to transform your files, ignoring "node_modules".

    Here's what you can do:
     • If you are trying to use ECMAScript Modules, see https://jestjs.io/docs/en/ecmascript-modules for how to enable it.
     • To have some of your "node_modules" files transformed, you can specify a custom "transformIgnorePatterns" in your config.
     • If you need a custom transformation specify a "transform" option in your config.
     • If you simply want to mock your non-JS modules (e.g. binary assets) you can stub them out with the "moduleNameMapper" config option.

    You'll find more details and examples of these config options in the docs:
    https://jestjs.io/docs/en/configuration.html

    Details:

    /Users/bennyp/Developer/storybook/scripts/jest.init.ts:8
    import "core-js/modules/es.array.includes.js";
    ^^^^^^

    SyntaxError: Cannot use import statement outside a module

      at Runtime.createScriptFromCode (node_modules/jest-runtime/build/index.js:1350:14)

Test Suites: 2 failed, 2 total
Tests:       0 total
Snapshots:   0 total
Time:        2.741 s
Ran all test suites matching /addons\/docs\/src\/frameworks\/web-components\//i

🤷‍♂️

[shilman]

@bennypowers doesn't seem to be working for me. what am i missing?

[bennypowers]

@shilman I think this is intended behaviour.

storybook/examples/web-components-kitchen-sink/src/components/sb-button.stories.ts

Lines 20 to 26 in 209cdac

	const Template: Story<SbButton> = ({ primary, backgroundColor, size, label }) =>
	html`<sb-button
	?primary="${primary}"
	.size="${size}"
	.label="${label}"
	.backgroundColor="${backgroundColor}"
	></sb-button>`;

TL;DR: only changes to primary will affect the source output because that's how the example story is written.

if you change those lines to the following (and similarly added background-color attribute to the descriptor for that property), then I think you'd see a source update.

const Template: Story<SbButton> = ({ primary, backgroundColor, size, label }) =>
  html`<sb-button
    ?primary="${primary}"
    size="${size}"
    label="${label}"
    background-color="${backgroundColor}"
  ></sb-button>`;

I've made those changes and updated this PR, so once CI is done you should see it reflected on the page

NOTE: there appears to be a problem with the argTypes derived from the manifest for the size attribute, which has a type that's a union of strings. I believe that is unrelated to this PR, though

[shilman]

@bennypowers i don't know enough about web-components to know what is expected behavior, but it looks off to me. naively, i would expect it to produce source strings like:

<sb-button
    ?primary=true
    size="large"
    label="hahaha"
    background-color="#ff0000"
  ></sb-button>

cc @gaetanmaisse

[bennypowers]

i would expect it to produce source strings like:

<sb-button
    ?primary=true
    size="large"
    label="hahaha"
    background-color="#ff0000"
  ></sb-button>

cc @gaetanmaisse

The purpose here is to output html that someone could copy and paste into any document, rather than, say, a lit-html template javascript source.

i don't know enough about web-components to know what is expected behavior

web components are one thing, and lit-html is another
the story originally applied DOM properties to the component, which wouldn't affect the output html unless the component was programmed to reflect those properties to attributes, which happens to be the case here with the primary property, however that was (originally) the only property assigned in the story as an attr.

so while it's true that storybook lumps "web components" in with "frameworks" by parsing lit-html templates, that's really just a bone that storybook tosses to story authors to make it easier to write stories. lit-html !== web components, not by a long shot, but the one thing each and every web component has in common is HTML and the DOM

[shilman]

isn't it strange, though, to see a picture of a large, red button that says "hahaha", and paste a snippet that results in a small, blue button that says "Button" (or whatever the default label is)?

i get the difference between lit-html and plain html. i think this examples illustrates part of the reason @brion-fuller and @gaetanmaisse have been pushing for a @storybook/lit package, where showing a lit snippet is perfectly self-consistent.

[bennypowers]

isn't it strange, though, to see a picture of a large, red button that says "hahaha", and paste a snippet that results in a small, blue button that says "Button"

I'm not sure I follow...
Given

If I paste the snippet into the preview html

I get

[shilman]

i was referring to the behavior before your last set of changes, which you described as expected. are you saying that people should only use lit-html attributes, not properties, if they want their snippets to reflect what's being rendered in the story? i could be ok with that, i just want to understand the limitations of this approach, since changing them from properties to attributes feels like a workaround.

[bennypowers]

i was referring to the behavior before your last set of changes, which you described as expected. are you saying that people should only use lit-html attributes, not properties, if they want their snippets to reflect what's being rendered in the story? i could be ok with that, i just want to understand the limitations of this approach, since changing them from properties to attributes feels like a workaround.

Yes*, I'm saying that this should be the "best practice" recommendation:

const Template: Story<SbButton> = ({ primary, backgroundColor, size, label }) =>
  html`<sb-button
    ?primary="${primary}"
    size="${ifDefined(size)}"
    label="${ifDefined(label)}"
    background-color="${ifDefined(backgroundColor)}"
  ></sb-button>`;

Which is in line with the general community standards - accept attributes if you can, properties for complex data in lieu of children. Components that follow those recommendations are more easily styled in <noscript> for example, and can pass between frameworks more easily.

Certainly here, where the goal is a copy-able snippet, content is king, so attrs should be the norm.

*Edit: It's hard to say "only" since I can't cover every use of every HTML element, but most users will probably want to do like ☝️ in most cases.

[Westbrook]

When I copied this approach locally, I saw what I expected from the output of the snippet, HTML that I can paste into another page and it just works. I’d prefer if it were context.originalStoryFn() so as to omit any decorators, but that might just be me.

It is possible that I missed the HTML updating with the Controls, but it is HTML that I’d want here so as not to require my consumers to work in my context, be that lit-html, JS string, etc. Assuming that’s true, I can’t fully check while remote from the beach this week, I’m a huge fan of this addition!

[shilman]

Thanks for the clarification @bennypowers @Westbrook. I agree it's a wonderful addition and I'm really excited about getting it merged!

I'd love it if @gaetanmaisse or @brion-fuller can take a look.

[Westbrook]

How do others feel about context.originalStoryFn here?

[shilman]

Here are my thoughts on the matter: #14652 (comment)

Since this is all brand new in web-components, we can make it opt-out OR opt-in, depending on how you guys feel. I believe it's safer to include the decorators by default and allow users to opt-out of the decorators. In the React world, it's quite common for stories to rely on the decorators to provide context to render properly. But I'll defer to your collective judgment for what's best in the web-components world.

[bennypowers]

@Westbrook can you elaborate on the case, both ways?

[Westbrook]

Yeah, I’m less worried as long as it’s configurable. However, for my library I use a very Storybook custom decorator to control theme color/scale/direction in a way that you wouldn’t want to require a copy/paster to clean up after the fact. storyFn() leaves all of the decorator code whereas without it you copy just the component under demo.

[shilman]

@bennypowers @Westbrook WDYT about adding configuration in this PR? i'm afraid if it doesn't get done now it will slip through the cracks.

[bennypowers]

I'm happy to put this in i'm just unfamiliar with the relevant apis.

is it just

const story = context.stripDecorators ? context.originalStoryFn() : storyFn(): 

??

[shilman]

Let's make it consistent with React:

const story = context?.parameters.docs?.source?.excludeDecorators
    ? context.originalStoryFn(context.args) 
    : storyFn();

[bennypowers]

Added with unit tests, thanks