sveltejs / kit

web development, streamlined
https://svelte.dev/docs/kit
MIT License
18.7k stars 1.94k forks source link

Inconsistent CSS positions for components between the dev and node adapter #7426

Open stalkerg opened 2 years ago

stalkerg commented 2 years ago

Describe the bug

I have global styles in the <svelte:head> for the main +layout.svelte. On the dev env, the components <styles> put after main <svelte:head> but adapter-node inject it before <svelte:head>. As you can understand, style and behavior depend on order, and some of my rules are not working as I expected.

Reproduction

Compare CSS position for dev and adapter-node relative <svelte:head>.

Logs

No response

System Info

System:
    OS: Linux 5.19 Gentoo Linux
    CPU: (24) x64 AMD Ryzen 9 5900X 12-Core Processor
    Memory: 5.70 GB / 31.26 GB
    Container: Yes
    Shell: 5.1.16 - /bin/bash
  Binaries:
    Node: 18.11.0 - /usr/bin/node
    npm: 8.19.2 - /usr/bin/npm
  npmPackages:
    @sveltejs/adapter-node: 1.0.0-next.99 => 1.0.0-next.99 
    @sveltejs/adapter-static: 1.0.0-next.46 => 1.0.0-next.46 
    @sveltejs/kit: 1.0.0-next.526 => 1.0.0-next.526 
    @sveltejs/svelte-virtual-list: github:sveltejs/svelte-virtual-list => 3.0.1 
    svelte: 3.52.0 => 3.52.0 
    vite: ^3.2.1 => 3.2.1

Severity

blocking all usage of SvelteKit

Additional Information

No response

stalkerg commented 2 years ago

Node adapter: Screenshot_20221029_100817

dev image

As you can see, on Node adapter, such CSS before the title wich in the head.

Rich-Harris commented 2 years ago

This will need a minimal reproduction if we're to create a test case and possible fix

stalkerg commented 2 years ago

Okey, I will try to do it ASAP.

stalkerg commented 2 years ago

@Rich-Harris okey, reproducing is very trivial:

  1. install sveltkit starter demo, npm create svelte@latest my-app
  2. run it in dev npm run dev, and as you can see, all CSS after <title>: изображение
  3. run it as preview npm run preview, and as you can see, all CSS before <title>: изображение

do you need something even simpler? I can try to do it or try to make a test case, but it seems like you can reproduce it in any trivial scenario.

Rich-Harris commented 2 years ago

This shows tags coming before/after the <title>, but to build a test case around this the styles themselves need to differ between prod and dev

stalkerg commented 2 years ago

We just need to add any <link to css in <svelte:head> and this will be illustrated, depending on before/after some rules will be overridden each over or not. I am currently moving into a new house and can probably make a unit/integration test only next week.

rodshtein commented 1 year ago

Screen Recording 2022-11-07 at 5 18 00 PM On preview above: right window is preview mode, left is dev

@Rich-Harris i made an simple reproduction https://stackblitz.com/edit/sveltejs-kit-template-default-g3jmhk?file=src/routes/+page.svelte

Main points:

stalkerg commented 1 year ago

In my opinion, the component CSS styles should have a chance to overwrite any styles from <svelte:head> and +layout.

Rich-Harris commented 1 year ago

The issue here is that when you import './styles.css', Vite injects the contents in a <style> element at the end of the <head>. So the SSR'd page looks like this...

<style><!-- imported styles injected by SvelteKit --></style>
<!-- <svelte:head> content, potentially including styles -->

...but after hydration, it looks like this:

<!-- <svelte:head> content, potentially including styles -->
<style><!-- imported styles injected by Vite --></style>

In prod, it looks like this:

<link rel="stylesheet" href="[bundled styles]">
<!-- <svelte:head> content, potentially including styles -->

The only way SvelteKit could make this consistent is by matching Vite's dev-time behaviour — placing imported .css files at the end of the <head>. But that seems rather undesirable. Perhaps if there was a way to tell Vite where to inject styles, these problems could be avoided. Beyond that, I'm not sure we have any great options.

stalkerg commented 1 year ago

hmm it's a huge stopper, in that case not possible to use any external CSS with SvelteKit especially if you must overwrite something.

Rich-Harris commented 1 year ago

Does this issue only happen if you're doing stuff in <svelte:head>? Because if so my best advice would be 'don't do that'

stalkerg commented 1 year ago

Yes, but how to support dynamic external CSS? The URL depends on the current user's theme choice.

<link href="{env.style_cdn_host}/{env.site_theme}.css?v=144" rel="stylesheet" type="text/css" />

I think I can implement by %mycssurl% replacement in app.html but in that case, a user must reload the full page, as I understand.

zyxd commented 1 year ago

Yes, but how to support dynamic external CSS? The URL depends on the current user's theme choice.

<link href="{env.style_cdn_host}/{env.site_theme}.css?v=144" rel="stylesheet" type="text/css" />

I think I can implement by %mycssurl% replacement in app.html but in that case, a user must reload the full page, as I understand.

This doesn't solve the problem in this topic, but for switching CSS themes loaded dynamically from external sources, you can do all the work in <head> rather than in the <style> of the component.

I mean, those styles that can be overwritten in the dynamically loaded theme should also be loaded via head > link.

https://stackblitz.com/edit/sveltejs-kit-template-default-ikbt4z

Rich-Harris commented 1 year ago

Yes, but how to support dynamic external CSS?

You could do it imperatively — add this to src/app.html...

<link id="theme" rel="stylesheet" href="REPLACE_ME_IN_HANDLE_HOOK">

...then do this sort of thing:

$: document.querySelector('theme').href = `${style_cdn_host}/${site_theme}.css?v=144`;

That way, you have complete control over where in the <head> the stylesheet goes, and with it the order in which stylesheets are applied

stalkerg commented 1 year ago

@Rich-Harris seems like working BUT looks awkward especially because I should share session data between +layout.server.js and hooks.server.js by a global variable (also it's probably can be the point of race condition).