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.

Sign up for GitHub

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

Jump to bottom

Web-components: Support lit 2 with back-compat #14898

Merged
merged 8 commits into from
May 25, 2021
Merged

Web-components: Support lit 2 with back-compat #14898

merged 8 commits into from
May 25, 2021

Conversation

Westbrook
Copy link
Contributor

Issue: refs #13489

What I did

Use split versions of lit-html and lit-element pointing to the pre-[email protected] versions that support the isTemplateResult() helper.

I checked out the @storybook/lit build, and I'm not sure what it's over all goals are, but having it separate seems like a weird play in the context of the x-compatibility story of web components, but happy to revisit if not.

How to test

  • Is this testable with Jest or Chromatic screenshots? By testing the @storybook/web-components examples.
  • Does this need a new example in the kitchen sink apps? Uses the @storybook/web-components examples.
  • Does this need an update to the documentation? Possibly?

If your answer is yes to any of these, please make sure to include it in your PR.

@nx-cloud
Copy link

nx-cloud bot commented May 12, 2021 •
edited
Loading

Nx Cloud Report

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

📂 See all runs for this branch

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

Sent with 💌 from NxCloud.

@shilman
Copy link
Member

shilman commented May 12, 2021

@gaetanmaisse @brion-fuller can explain more fully, but the rough goals of the @storybook/lit project is first-class support for lit2 including some features not supported in web-components such as:

  • addon-docs dynamic snippet support
  • HMR support
  • modern custom-elements support
  • plus a variety of details such as args spreading

The rationale behind the rename is to make the lit connection more explicit. Just like you can use @storybook/html to write stories for web-components, you can also use lit. The idea is to phase out @storybook/web-components in favor of @storybook/lit or other variants (e.g. @storybook/stencil). If you think this is a bad direction, I'd love to discuss. We're still in early alpha with it, and nothing's set in stone yet.

@anoblet
Copy link

anoblet commented May 18, 2021 •
edited
Loading

@shilman Is there any way we could merge this for the time being, until @storybook/lit is considered production ready?

@shilman
Copy link
Member

shilman commented May 18, 2021

@anoblet we're meeting with @Westbrook @daKmoR etc on thursday to figure out a path forward on this

@anoblet anoblet mentioned this pull request May 18, 2021
Closed
@gaetanmaisse gaetanmaisse self-assigned this May 21, 2021
@shilman shilman changed the title feat: support both before and after [email protected] in the web components app Web-components: Support both before and after [email protected] May 21, 2021
@shilman shilman changed the title Web-components: Support both before and after [email protected] Web-components: Support lit 2 w/ back-compat May 21, 2021
@shilman shilman changed the title Web-components: Support lit 2 w/ back-compat Web-components: Support lit 2 with back-compat May 21, 2021
@gaetanmaisse
Merge branch 'next' into lit-next
9d3d038
@gaetanmaisse
fix(web_components): keep .js extension to avoid issue with Webpack…
c42ba7c 
… (related to export map?)
@gaetanmaisse
feat(cli): use @storybook/web_components when running sb init in a …
c0edc6e 
…project with `lit` as dep
@gaetanmaisse
feat(cli): add a repro config for lit 2
0fe0852
@gaetanmaisse
feat(cli): do not add lit-html automatically when running sb init
e83cdc6 
This SB generator is used in 3 cases and for each `lit-html` is already there so no need to reinstall it:
 - User's project has `lit-html` as dep -> ok nothing to do
 - User's project has `lit-element` as dep -> ok as `lit-html` is a dep of `lit-element`
 - User's project has `lit` as dep -> ok as `lit-html` is a dep of `lit-element`

Note: SB WC will not work out of the box with Yarn in PnP mode as SB is doing import from `lit-html` directly.
For now the recommended work around is to add `lit-html` as dep to satisfy SB WC peer dependency.
We will be able to simplify/fix that easily when `lit-*` will be deprecated in favor of `lit`.
@gaetanmaisse
e2e: add Lit 2 + TypeScript config as part of the core e2e tests
b9ec80a
@gaetanmaisse
test: mock lit-html/directive-helpers.js module in addons docs tests
10e1fab
gaetanmaisse
gaetanmaisse approved these changes May 24, 2021
View reviewed changes
Copy link
Member

@gaetanmaisse gaetanmaisse left a comment

Choose a reason for hiding this comment

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

🚀 LGTM

@shilman I updated a bit the e2e tests to have both lit 1 + 2, it also means we have a repro config for Lit 2. Before any public announcement, I would like to make more test with both Lit 1 + Lit 2 based projects (and try to migrate my company project 😉 ).

Note: SB WC + Lit 2 will not work out of the box with Yarn in PnP mode as SB is doing imports from lit-html directly. For now, the recommended workaround is to add lit-html as dep to satisfy SB WC peer dependency. We will be able to simplify/fix that easily when lit-* will be deprecated in favour of lit.

@gaetanmaisse gaetanmaisse assigned shilman and unassigned gaetanmaisse May 24, 2021
shilman
shilman approved these changes May 25, 2021
View reviewed changes
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

Looking great @Westbrook @gaetanmaisse 💯

@shilman shilman merged commit 2884c0c into storybookjs:next May 25, 2021
@Westbrook Westbrook deleted the lit-next branch May 25, 2021 00:51
This was referenced May 25, 2021
Closed
Closed
@gaetanmaisse
Copy link
Member

Note: Depending on the package manager used and the dependency hoisting it can be necessary to add lit-html 2.x to your dev dep to make Storybook Web Components work with Lit 2. I will be working on improving that soon.

@43081j 43081j mentioned this pull request Jun 17, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gaetanmaisse gaetanmaisse gaetanmaisse approved these changes

@shilman shilman shilman approved these changes

@ndelangen ndelangen Awaiting requested review from ndelangen

@stijnkoopal stijnkoopal Awaiting requested review from stijnkoopal

Assignees

@shilman shilman

Labels
feature request web-components
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Westbrook @shilman @anoblet @gaetanmaisse