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

Plugin is incompatible with Storybook using Vite@2.9.7 #50

Closed
troyvassalotti opened this issue May 4, 2022 · 10 comments
Closed

Plugin is incompatible with Storybook using Vite@2.9.7 #50

troyvassalotti opened this issue May 4, 2022 · 10 comments

Comments

@troyvassalotti
Copy link

With vite's update to v2.9.7, the plugin's default settings no longer work when used in conjunction with @storybook/builder-vite.

A minimal reproducible example can be found here: https://github.com/troyvassalotti/storybook-vite-plugin-mre with steps on how to reproduce and the error that is happening.

It seems that Vite may have changed how CSS imports work and it is creating an issue when trying to run a Storybook dev server. This issue is related to #49. In order for the plugin to work with Storybook so we can import SASS into our Lit components, we need to specifically include only SASS files. We lose the option to include standard CSS files after the Vite update.

@IanVS
Copy link

IanVS commented Jun 21, 2022

I've been digging into why our example lit project has started failing in the storybook builder when updating vite to v3.

One thing I've noticed so far, is that the code from vite now looks like this:

export default /* #__PURE__ */ (() => "p{color:#00f}\n")()

Which might not be what this plugin is expecting. Here's the node structure:

Node {
  type: 'ExportDefaultDeclaration',
  start: 0,
  end: 58,
  declaration: <ref *1> Node {
    type: 'CallExpression',
    start: 31,
    end: 58,
    callee: Node {
      type: 'ArrowFunctionExpression',
      start: 32,
      end: 55,
      id: null,
      expression: true,
      generator: false,
      async: false,
      params: [],
      body: [Node],
      parent: [Circular *1],
      edit: [Helpers],
      getSource: [Function: bound source],
      update: [Function: bound update],
      source: [Function: bound source],
      append: [Function: bound append],
      prepend: [Function: bound prepend]
    },
    arguments: [],
    optional: false,
    _rollupAnnotations: [ [Object] ],
    parent: [Circular *2],
    edit: Helpers { node: [Circular *1], string: {} },
    getSource: [Function: bound source],
    update: [Function: bound update],
    source: [Function: bound source],
    append: [Function: bound append],
    prepend: [Function: bound prepend]
  },

But, this plugin is looking for a node.declaration.name and node.declaration.type === 'Literal'

IanVS added a commit to storybookjs/builder-vite that referenced this issue Jun 21, 2022
IanVS added a commit to storybookjs/builder-vite that referenced this issue Jun 22, 2022
IanVS added a commit to storybookjs/builder-vite that referenced this issue Jun 27, 2022
@bluwy
Copy link

bluwy commented Jun 27, 2022

It's likely caused by vitejs/vite#8278 (Vite 3) and vitejs/vite#8471 (Vite 2 backport). I'm not sure if the plugin is tapping into Vite's internals to be a regression, but the PRs were improvements to reduce hacks made in the codebase.

@umbopepato
Copy link
Owner

Thanks for investigating @IanVS! I'll try and handle that AST format as well.
What surprises me is that in @troyvassalotti's example the css code is sent to the plugin as raw unprocessed text...

@bluwy
Copy link

bluwy commented Jul 1, 2022

Note The linked PRs has been reverted due to other regressions caused. See vitejs/vite#8874

IanVS added a commit to storybookjs/builder-vite that referenced this issue Jul 8, 2022
@jrencz
Copy link

jrencz commented Dec 27, 2022

Just for the record: adjusting default include pattern solved the problem for me

include: '**/*.{css,sss,pcss,styl,stylus,sass,scss,less}',

include: '**/*.{css,sss,pcss,styl,stylus,sass,scss,less}?(*)'

Maybe a way to go would be to introduce an option allowing to match without query (and use it by default)

@umbopepato
Copy link
Owner

Cool @jrencz, let's change the default include to that one then. Feel free to to open a PR if you want!
As for the additional option, I'd say changing the include option directly should be enough, no?

@jrencz
Copy link

jrencz commented Dec 28, 2022

It should, but what I meant was: instead of matching whole id, let's instead extract the path out of it and filter the paths. That would make it impossible to match query if one wanted to, as one can now

Additional flag/option would allow users to revert to the native way (i.e. with query) if needed.

@umbopepato
Copy link
Owner

umbopepato commented Jun 21, 2023

Is anyone still experiencing this problem? I cannot reproduce it with recent versions of vite and @storybook/builder-vite. Anyway, just to be sure, I added support for template literals and iife expressions (and combinations of those) in v2.1.0

@sr258
Copy link

sr258 commented Oct 30, 2023

@umbopepato I've recently had the issue when using Storybook 7.4 and rollup-plugin-postcss-lit 2.0.0. I've updated to Storybook 7.5 and rollup-plugin-postcss-lit 2.1.0 and the issue has disappeared! Thanks for fixing it!

@umbopepato
Copy link
Owner

Awesome @sr258, thank you for the feedback! 😊

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

No branches or pull requests

6 participants