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

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

Open
NikolayMakhonin opened this issue Jan 24, 2020 · 6 comments
Milestone

Comments

@NikolayMakhonin
Copy link

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 Conduitry added the bug label Jan 24, 2020
@Conduitry
Copy link
Member

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.

colincasey added a commit to colincasey/svelte that referenced this issue Jan 27, 2020
This is an attempt to fix sveltejs#4313 which demonstrates that incorrect css can be produced when two components have the same style rules and the first component to be included has unused selectors.
@Conduitry
Copy link
Member

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
Copy link

fkling commented Dec 18, 2020

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
Copy link

stale bot commented Jun 26, 2021

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
Copy link
Author

@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
Copy link
Author

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,
      ...
    }),
    ...
  ],
  ...
}

@Rich-Harris Rich-Harris added this to the 5.0 milestone Apr 2, 2024
@Rich-Harris Rich-Harris modified the milestones: 5.0, 5.x Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants