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

vite-plugin-svelte chokes on custom media syntax when using vitePreprocess and Lightning CSS #733

Closed
knpwrs opened this issue Sep 4, 2023 · 8 comments · Fixed by #756
Closed
Labels
bug Something isn't working has-workaround

Comments

@knpwrs
Copy link

knpwrs commented Sep 4, 2023

Describe the bug

I just spun up a new skeleton sveltekit project which by default uses vitePreprocess. I installed lightning css (npm i -D lightningcss) I have modified vite.config.ts to contain the following:

import { sveltekit } from '@sveltejs/kit/vite';
import { defineConfig } from 'vite';

export default defineConfig({
  plugins: [sveltekit()],
  css: {
    transformer: 'lightningcss',
    lightningcss: {
      drafts: {
        nesting: true,
        customMedia: true,
      },
    },
  },
});

Given the following +page.svelte file, I can see the nesting configuration works:

<h1>Welcome to SvelteKit</h1>

<style>
  /* @custom-media --small-viewport (max-width: 30em); */

  h1 {
    color: blue;

    &:hover {
      color: red;
    }

    /* @media (--small-viewport) { */
    /*   color: green; */
    /* } */
  }

</style>

With the configuration set as in my first code block, hovering the <h1> turns it red. If I comment out the nesting configuration, hovering the <h1> does nothing.

However, if I uncomment the custom media, I get a syntax error:

Internal server error: /Users/knpwrs/Downloads/my-app/src/routes/+page.svelte:4:43 ")" is expected
  Plugin: vite-plugin-svelte
  File: .../my-app/src/routes/+page.svelte:4:43
   2 |
   3 |  <style>
   4 |    @custom-media --small-viewport (max-width: 30em);
                                                   ^

Reproduction URL

https://github.com/knpwrs/sveltekit-lightningcss-custom-media-issue

Reproduction

  1. npm create svelte@latest my-app
  2. cd my-app
  3. npm install -D lightningcss
  4. Update configuration as shown above
  5. Observe bug

Logs

logs.txt

System Info

System:
    OS: macOS 13.4
    CPU: (12) arm64 Apple M2 Max
    Memory: 1.26 GB / 64.00 GB
    Shell: 5.9 - /opt/homebrew/bin/zsh
  Binaries:
    Node: 20.3.1 - /opt/homebrew/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 9.6.7 - /opt/homebrew/bin/npm
  Browsers:
    Chrome: 116.0.5845.140
    Chrome Canary: 117.0.5863.0
    Safari: 16.5
  npmPackages:
    @sveltejs/adapter-auto: ^2.0.0 => 2.1.0
    @sveltejs/kit: ^1.20.4 => 1.24.0
    svelte: ^4.0.5 => 4.2.0
    vite: ^4.4.2 => 4.4.9
@knpwrs knpwrs added bug Something isn't working triage Awaiting triage by a project member labels Sep 4, 2023
@dominikg
Copy link
Member

dominikg commented Sep 5, 2023

The issue is caused here:

if (!supportedStyleLangs.includes(lang)) return;

your style block does not have a "lang" attribute and then vitePreprocess skips it entirely. You can work around it with the current version of vite-plugin-svelte by setting lang="css".

@bluwy any reason not to fall back to 'css' if lang isn't set?

@dominikg dominikg added has-workaround and removed triage Awaiting triage by a project member labels Sep 5, 2023
@knpwrs
Copy link
Author

knpwrs commented Sep 5, 2023

That works! It's even possible to put custom media queries in a separate file and import it e.g., @import 'media.css'; or @import '$lib/media.css';.

Thank you! Looking forward to seeing how this is resolved long term.

@bluwy
Copy link
Member

bluwy commented Sep 6, 2023

I think it was to keep it compatible with svelte-preprocess before, which only preprocesses things with the lang attribute. The issue is similar to postcss not applying its transform because you don't have lang="postcss" set.

Technically Vite always run postcss or lightningcss on all CSS files though, so maybe in the next v-p-s major we can change to always run the preprocessing.

@dominikg
Copy link
Member

dominikg commented Sep 6, 2023

great point about this being a breaking change and also mentioning lang="postcss".

Makes sense to have vitePreprocess process all style nodes with supported langs, but this is going to make it more different from svelte-preprocess. I wonder if we need to document those differences to avoid confusion for users.

cc @kaisermann

@knpwrs
Copy link
Author

knpwrs commented Sep 8, 2023

I'm not sure what the interaction between svelte-check and vite-plugin-svelte is, but lang="css" appears to cause linting issues with custom media:

❯ npm run check

> [email protected] check
> svelte-kit sync && svelte-check --tsconfig ./tsconfig.json


====================================
Loading svelte-check in workspace: /Users/knpwrs/Workspace/letschurch/lets.church/apps/web-svelte
Getting Svelte diagnostics...


/Users/knpwrs/Workspace/letschurch/lets.church/apps/web-svelte/src/routes/+page.svelte:5:3
Warn: Unknown at rule @custom-media (css)
<style lang="css">
  @custom-media --small-viewport (max-width: 30em);



====================================
svelte-check found 0 errors and 1 warning in 1 file

If I set lang="postcss" that warning does not appear (even though I don't have postcss configured at all, nevermind a custom media plugin for postcss):

❯ npm run check

> [email protected] check
> svelte-kit sync && svelte-check --tsconfig ./tsconfig.json


====================================
Loading svelte-check in workspace: /Users/knpwrs/Workspace/letschurch/lets.church/apps/web-svelte
Getting Svelte diagnostics...


====================================
svelte-check found 0 errors and 0 warnings

Would this warrant a new issue in this repo or another? Or would a fix here also fix the svelte-check issue?

This behavior reproduces with the LSP as well:

image image

@dominikg
Copy link
Member

dominikg commented Sep 8, 2023

good question. i assume it still preprocesses the style block with lightningcss if you set lang="postcss" in your code? if yes that would be the workaround for that one.

Maybe we should add lang="lightningcss" too ?

cc @dummdidumm

@knpwrs
Copy link
Author

knpwrs commented Sep 8, 2023

I hadn't even thought to try that, but yes indeed, that does work and lightningcss still processes the style block.

@kaisermann
Copy link
Member

I think it was to keep it compatible with svelte-preprocess before, which only preprocesses things with the lang attribute. The issue is similar to postcss not applying its transform because you don't have lang="postcss" set.

@bluwy in theory, if postcss is configured, svelte-preprocess will run postcss on style blocks without the lang attribute.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working has-workaround
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants