unocss / unocss

The instant on-demand atomic CSS engine.
https://unocss.dev
MIT License
16.49k stars 833 forks source link

svelte scoped: preflights not applied when using preprocessor only #2801

Closed henrikvilhelmberglund closed 1 year ago

henrikvilhelmberglund commented 1 year ago

UnoCSS version

0.53.4

Describe the bug

Preflights are not applied when using the preprocessor only. Basically https://github.com/unocss/unocss/issues/2629. I saw the nice onlyGlobal thing using the Vite plugin, maybe that's the way to go, but if preflights are supposed to work with only the preprocessor it is a bug.

I could also be doing something wrong but that should hopefully be pretty obvious with the repro below.

Reproduction

https://stackblitz.com/edit/unocss-unocss-y3ienz?file=src%2Froutes%2F%2Bpage.svelte @jacob-8 here is the repro!

System Info

No response

Validations

jacob-8 commented 1 year ago

Svelte 4 broken: https://stackblitz.com/edit/unocss-unocss-ea8szj?file=src%2Froutes%2F%2Bpage.svelte Svelte 3 works: https://stackblitz.com/edit/unocss-unocss-u3yk8b?file=src%2Froutes%2F%2Bpage.svelte

Svelte 4 does preprocessors a little differently, but the docs still indicate we can use the attributes property (https://svelte.dev/docs/svelte-compiler#types-preprocessor) though I think in Svelte 5 that may have to be parsed manually as they indicated they are moving towards simplifying and might just provide optional helper functions for stuff like that. So I'm not sure why it's broken currently, but now we know what change caused it.

As a by the way, I guess the docs are not clear enough but uno:preflights is designed to be added just to the specific component where it is needed as the preprocess option is designed for those building individual components that are consumed as needed. It is not designed to be used globally as seen in your repro. Use the Vite plugin for those situations.

Also, do not import the reset files into your root layout file. As you can see from this example here, that does not guarantee they come first and the order of CSS imports by Svelte components is actually different between server and client in some cases.

image

Resets must be imported into your head tag before %sveltekit.head% as done by Vite plugin or you must write them in manually.

jacob-8 commented 1 year ago

I logged out the attributes object when running the test against https://github.com/unocss/unocss/tree/main/packages/svelte-scoped/test/cases/preflights/Input.svelte and found the explanation:

image

Using : in a custom attribute is no longer allowed in Svelte 4: https://github.com/sveltejs/svelte/pull/8633 so we'll have to switch to uno-preflights and uno-safelist.

henrikvilhelmberglund commented 1 year ago

Thanks for the info about uno:preflights, I've been using the preprocessor option with mdsvex so not really creating a library so the unocss config options confused me a bit.

I don't really understand the part about the global +layout.svelte though, I thought that was the way you imported global CSS now in SvelteKit. Without SSR the reset is always loaded first for me (of the <style type="text/css" files) and with SSR it is included inline in the rendered file itself.

Or do you mean that other things in app.html will load before the reset? What problems would that cause? And how should I import it in the app.html if I don't want to use the Vite plugin?

Sorry for a lot of questions 😊

jacob-8 commented 1 year ago

No need to apologize, @henrikvilhelmberglund - you're my best bug finder!

I've been using the preprocessor option with mdsvex

Why would using mdsvex require using the preprocessor version of svelte-scoped? I use mdsvex on a few projects and I don't see any reason why you can't still use the @unocss/svelte-scoped/vite plugin.

I don't really understand the part about the global +layout.svelte though, I thought that was the way you imported global CSS now in SvelteKit. Without SSR the reset is always loaded first for me (of the <style type="text/css" files) and with SSR it is included inline in the rendered file itself.

Still to be documentated, SvelteKit does not try to guarantee style imports ordering - see https://github.com/sveltejs/kit/issues/7751 - so stay away from any situation where you depend on that if you want to avoid flashes of change between server and client hydration as seen in my repro: https://stackblitz.com/edit/sveltejs-kit-template-default-ceqnem

If you don't use svelte-scoped Vite plugin then another way to do resets is just to import a standard reset in your app.html like this for example: <link rel="stylesheet" href="https://unpkg.com/normalize.css@8.0.1/normalize.css" />

henrikvilhelmberglund commented 1 year ago

Do you mean you use both the Vite plugin and the preprocessor or just the Vite plugin? When I try to use only the Vite plugin I don't get any styles at all. I don't have a small repro for this but made a branch in a project if you would like to see how it looks: https://github.com/henrikvilhelmberglund/Tutorials/tree/vite-plugin-style

If I wanted to use import '@unocss/reset/tailwind.css' in app.html, would I need to copy that file from node modules to /static and then add it like this <link rel="stylesheet" href="%sveltekit.assets%/tailwind.css" /> or can I do it straight from node modules?

jacob-8 commented 1 year ago

When I try to use only the Vite plugin I don't get any styles at all.

This is because you are setting it up using instructions from unocss/vite and not @unocss/svelte-scoped/vite - get your intellisense and IDE type-checker working and it will catch bugs like this for you automatically. Intellisense shows you the options. Config must be placed inside configOrPath or in your uno.config.js file which you left empty.

image

The transformers only apply to CSS files, so that has an updated property name outside of the configOrPath (see docs), and there is no filter property because that's the responsibility of your svelte.config.js to define. All Svelte files will be transformed.

Have a read through the docs again and please let me know if there are any points that need clarified there. I think most of them would have been cleared up the intellisense documentation though. Your working code can be seen at: https://stackblitz.com/edit/github-namyab?file=vite.config.js

If I wanted to use import '@unocss/reset/tailwind.css' in app.html, would I need to copy that file from node modules to /static and then add it like this <link rel="stylesheet" href="%sveltekit.assets%/tailwind.css" /> or can I do it straight from node modules?

Just like you have there is how you have to do it. Neither Vite nor SvelteKit will look inside of your app.html and place imports into the static folder for you. You have to manually do it and then import it as your wrote.

henrikvilhelmberglund commented 1 year ago

Thanks for taking a look! The include thing and transformers are just old code that I didn't really need. The empty uno.config.js is just to enable the extension, I don't have my actual config in there. If that file doesn't exist the extension can't find a valid config.

I saw that configOrPath: { } is required which made me a bit confused. Is this only required when using the svelte-scoped Vite plugin and why was it added? My other projects (for now using preprocessor) doesn't have this line even though when hovered it says UnocssSveltePreprocessOptions.configOrPath which seems like it should be used for the preprocess option and not the Vite plugin option, also it isn't mentioned in the docs at all.

Just like you have there is how you have to do it. Neither Vite nor SvelteKit will look inside of your app.html and place imports into the static folder for you. You have to manually do it and then import it as your wrote.

Got it, thanks!

jacob-8 commented 1 year ago

I saw that configOrPath: { } is required which made me a bit confused. Is this only required when using the svelte-scoped Vite plugin and why was it added

It is not required as you can see from the ? after it in my screenshot. It is there for those who prefer to inline while also allowing for other options specific to just svelte-scoped to be passed in. I personally prefer to use uno config.ts for the reason you mentioned, the extension, but I was just going with your code. I recommend however that you place your uno config portion into its file and leave just the svelte-scoped stuff in the vite config.

henrikvilhelmberglund commented 1 year ago

What I meant is that if I want to use an inline config in vite.config.js I need to use configOrPath: { } for it to work which is different from any other UnoCSS inline configs where you just have the config inline.

I guess the easiest solution is to just use uno.config.js instead but the fact that inline options in vite.config.js should be in configOrPath: { } should be documented somewhere.

in my opinion instead of this

UnoCSS({
    classPrefix: "uno",
    injectReset: "@unocss/reset/tailwind.css",
    configOrPath: {
        presets: [presetUno()],
    },
}),

something like this would make more sense to me:

UnoCSS({
    svelteScoped: {
        classPrefix: "uno",
        injectReset: "@unocss/reset/tailwind.css",
    },
    presets: [presetUno()],
}),

because it would be more compatible with other config styles. No idea if something like this would actually be possible though.

jacob-8 commented 1 year ago

Sure, I hear you. Good thoughts, but I take a different view.

Each integration has carved their own path, and the only unifying thread is that they all encourage users to use uno.config.ts.

Personally, I think the main Vite plugin's choice is due to convenience by whomever wrote it that way, but to me it is actually less "compatible" because properties related to just the plugin and properties relating to all of Uno can be intermixed, making copy-pasting config chunks a bit harder.

As seen in the examples and the documentation of each integration, Uno config is encouraged to be put in the dedicated uno.config.ts. So to make svelte-scoped options only accessible via a nested property would inconvenience the encouraged use pattern.

the fact that inline options in vite.config.js should be in configOrPath: { } should be documented somewhere.

Since the intellisense is the documentation and we are not trying to encourage people to use configOrPath, I think you can understand now why we don't highlight it's presence in the docs. The docs are already a bit long and other things are more important. That option is just for power users who for some reason need to place their uno.config.ts in a different location or who want to inline to get the Vite server to restart on each config change.

henrikvilhelmberglund commented 1 year ago

I think using the uno.config.js is fine really, not sure why I didn't use it before. And to be honest any problem like this will only happen if you don't use the uno.config.js.

Good point about the other frameworks, I didn't think that you needed two configs, one for UnoCSS itself and one for the framework integration, so the fact that you can use only one config inline in the Vite project is a bit of an exception.

I guess this is a bug then though because not passing in a preset shouldn't result in no styling at all, the default preset (presetUno) should be loaded anyway. You can repro this by removing everything inside defineConfig({}) in the Svelte scoped Vite project example vs the SvelteKit Vite project example and comparing the results.

jacob-8 commented 1 year ago

You can repro this by removing everything inside defineConfig({}) in the Svelte scoped Vite project example vs the SvelteKit Vite project example and comparing the results.

Please don't think of @unocss/svelte-scoped/vite as a clone of @unocss/vite with just a few changes to scope things. That was the previous situation when it was incorporated into the original Vite plugin just as a mode. The reason for the split was because many things didn't work in a svelte-scoped context (like the dev tools plugin) or are just unused bloat (custom extractors - this wouldn't work in svelte-scoped; rollup filters - svelte.config.js already takes of this, etc...). @unocss/vite has a whole different pipeline set up for finding styles, queueing tasks, caching results, etc... that can't be done the same way in our context.

It would help if you think of @unocss/svelte-scoped/vite as its own thing, built because @unocss/vite could not provide what was needed and had too much other stuff that went unused. The goal was a clean reset which makes sure all included code is needed. Therefore whatever features it lacks that you find elsewhere, please know that PRs are very welcome. Here's some that I know off the top of my head that would be nice:

As an interesting endeavor, I highly encourage you to read the source code for @unocss/vite - you'll see what I mean about many, many useful pieces of code that are great in that context but useless in svelte-scoped. You would also find starting points for each of the above nice-to-haves, and understand how to build them but also why a straight copy-paste is not possible.

Also why don't you try adding a default config for us and testing it out? Open https://github.com/unocss/unocss/main/tree/packages/svelte-scoped/src/_vite/index.ts, add this:

import { type UserConfigDefaults } from '@unocss/core' // merge with same import
import presetUno from '@unocss/preset-uno'

// add this
const defaults: UserConfigDefaults = {
  presets: [
    presetUno(),
  ],
}

function createSvelteScopedContext(configOrPath?: UserConfig | string): SvelteScopedContext {
    ...
    uno.setConfig(config, defaults) // add `defaults` here
    ...
}

Install packages, then run stub or build command, cd into the appropriate example, install packages, make needed adjustments to try it and then run the dev server for the example.

henrikvilhelmberglund commented 1 year ago

I think I just assumed it would just work like the other stuff but it makes sense that it has its own way of working now.

I would like to get into some more development actually and do something else than just creating issues to annoy people, I don't know a lot of things though like how to use my own version of the package rather than the one in package.json, ESLint setup and so on. I'll give it a try though.

Anyway I'll stop bothering you now. Thanks again! 😊