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

Dynamic class breaks CSS selector #3544

Closed
jesseskinner opened this issue Sep 10, 2019 · 3 comments
Closed

Dynamic class breaks CSS selector #3544

jesseskinner opened this issue Sep 10, 2019 · 3 comments
Labels

Comments

@jesseskinner
Copy link
Contributor

Describe the bug
Some style rules aren't being applied when there is a dynamic class between the root node and target node of the selector.

Logs
N/A

To Reproduce
https://svelte.dev/repl/c122185d662a4897a4cda657d07a3027?version=3.12.0

Expected behavior
I would expect all three lines of the REPL to have red text. Only the second and third do.

Stacktraces
N/A

Information about your Svelte project:
Firefox 69.0
Arch Linux
Svelte 3.12.0

Severity
It's annoying and very unexpected, but I can work around it.

Additional context
N/A

@Conduitry Conduitry added the bug label Sep 10, 2019
@hbendev
Copy link

hbendev commented Sep 10, 2019

The problem here is that, the container div doesn't get the svelte-generated class name. The child's class can be any variable declared in a script tag too, and an {#if} tag also causes the same problem, so I guess it can be reproduced with any svelte-tag.

<div class="root">
	{#if true}
		<div class="{aNumber}">
			<div>a number</div>
		</div>
	{/if}
</div>

results in
<div class="root"><div class="24"><div>a number</div></div></div>

@Cleecanth
Copy link
Contributor

Not a fix, but a "simple" workaround is to add an empty .root{} declaration to your style block. This forces Svelte to add a generated class name to the first div.

Thankfully, the blank selector should be easily removed by a minifier while still maintaining the generated class.

jesseskinner added a commit to jesseskinner/svelte that referenced this issue Oct 2, 2019
@jesseskinner
Copy link
Contributor Author

As you can see I've added a test for the issue in my fork. I'm very interested in trying to fix this issue, if it's okay with everyone?

I can identify a contradiction in the current code (Selector.ts) that will require a significant change to how encapsulation is calculated in order to fix this issue:

  1. A maximum of two nodes (ie. the first and last) are encapsulated per Selector
  2. When possible, only the outermost matching nodes of a selector are encapsulated, so we can usually omit the svelte-xxxxx class from the inner matching nodes of a selector.
  3. More than two nodes could require encapsulation for a single Selector

I plan to do some refactoring of apply_selector and add additional tests to make this possible without breaking anything. The cheap way out would be to add the svelte class in more cases, but my goal is to do better than this and maintain the existing behaviour when possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants