sveltejs / svelte

Cybernetically enhanced web apps
https://svelte.dev
MIT License
77.41k stars 4.03k forks source link

CSS isolation bug when components styles is equals and have not used css rule #4313

Open NikolayMakhonin opened 4 years ago

NikolayMakhonin commented 4 years ago

I think it is optimization side effect. Likely it can be reproduced only if you have at least one not used css rule. See this repl

See these components:

Component1.svelte

<div class="red"> Red </div>

<style>
    .red {
        background: red;
    }
    .yellow {
        background: yellow;
    }
</style>

Component2.svelte

<div class="red"> Red </div>
<div class="yellow"> Yellow </div>

<style>
    .red {
        background: red;
    }
    .yellow {
        background: yellow;
    }
</style>

1. For these components svelte generates different CSS with same css class suffixes

Component1.css

.red.svelte-1q75qj7{
    background:red
}

Component2.css

.red.svelte-1q75qj7 {
    background:red
}
.red.svelte-1q75qj7{
    background:red
}
.yellow.svelte-1q75qj7{
    background:yellow
}

2. Depending on which component loads first, only one of these styles will be loaded. You can see in the repl that Yellow div is not yellow. Switch the tabIndex default value to 2, and this div will be yellow.

App.svelte

<script>
    import Component1 from './Component1.svelte'
    import Component2 from './Component2.svelte'

    let tabIndex = 1
</script>

<button on:click={() => tabIndex = 1}>Tab 1</button>
<button on:click={() => tabIndex = 2}>Tab 2</button>

{#if tabIndex === 1}
    <Component1 />
{:else}
    <Component2 />
{/if}

3. Even if both styles will be loaded, their loading order will affect the style of the component. Thus, the styles of some components will affect others.

Conduitry commented 4 years ago

Interesting. I guess the first way I'm thinking about to handle this is to compute the hash based on the CSS with the unused selectors already removed. (Right now it's a hash of the literal string inside the <style> tag.) If that's going to be too much fiddly work or require too many changes, we could just have it be a hash of the styles concatenated with a list of the removed selectors, since it doesn't matter what sort of string it's the hash of.

Conduitry commented 4 years ago

To preserve the comment from the closed PR, @tanhauhau suggested basing the CSS hash class on the entire component's string, not just the styles. This would certainly work, and is how it worked before #1091. Some of the reasoning for that change was in the now-deleted Gitter channel, but I think it had something to do with writing a component that you rollup-plugin-replace bits of the HTML of to handle localization, and to let this not affect the generated CSS.

fkling commented 3 years ago

I'm also running into this issue. My use case is that I'm dynamically generating multiple components based on a shared template (which contains <script> and <style>), but each component has different content.

My current workaround is to inject the unique name of the component into a comment in the CSS.

Though it looks like if the CSS is emitted as a separate file, then the styles work regardless in which order the components are loaded.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

NikolayMakhonin commented 1 year ago

@Conduitry The bug is still reproduced in v3.52.0. This is a critical bug, svelte must guarantee the style isolation. Maybe it's time to fix it? We can simply add svelte file path to the hash function of the svelte-{hash} class.

https://svelte.dev/repl/6474b1394fa94fb1af2ef3445c5ef96f?version=3.52.0

NikolayMakhonin commented 1 year ago

Temporary solution:

rollup.config.js

function normalizePath(filepath) {
  return filepath.replace(/\\/g, '/')
}

const preprocessBase = sveltePreprocess({
  postcss   : true,
  typescript: true,
})

const preprocess = {
  ...preprocessBase,
  async style({content, filename, ...others}) {
    const result = await preprocessBase.style({
      content,
      filename,
      ...others,
    })

    // fix the style isolation bug: https://github.com/sveltejs/svelte/issues/4313
    result.code = `/* ${normalizePath(filename)} */\r\n${result.code}`

    return result
  },
}

export default {
  ...
  plugins: [
    ...
    svelte({
      preprocess,
      ...
    }),
    ...
  ],
  ...
}