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

Spread and scoping class problems #3790

Closed
Conduitry opened this issue Oct 25, 2019 · 0 comments · Fixed by #3792
Closed

Spread and scoping class problems #3790

Conduitry opened this issue Oct 25, 2019 · 0 comments · Fixed by #3792
Labels

Comments

@Conduitry
Copy link
Member

Conduitry commented Oct 25, 2019

<style>
	div { color: red; }
	.bar { font-weight: bold; }
</style>

<div {...{ class: 'bar' }} class='foo'>
	This is red.
</div>

<div class='foo' {...{ class: 'qux' }}>
	This should be red.
	It's not because the spread clobbers the scoping class attached to the class='foo'.
</div>

<div {...{ class: 'bar' }}>
	This is red, but should also be bold.
	It's not because the scoping class clobbers the class='bar'.
</div>

The style scoping class doesn't play nicely with spreads. There are a couple bad things that can happen, as shown above. There's at least one existing issue that this one intends to supersede, as well as probably some other as-yet-unreported issues.

It seems to me the proper way to handle this is to (at least in the case where we have spreads, possibly all the time, not sure) stop adding the scoping class by tweaking the AST so it has a new/updated class= attribute.

Instead, we can have a needs_scoping boolean on the Element, and when we're rendering the DOM code for this element, just do something like an elm.className += ' svelte-xyz';. I'd have to check whether this does the right thing if there's no class.

In SSR mode, we're going to have to do something else, because you can't just programmatically add a class to an element after the fact when you're writing a string of HTML as you go. Maybe another argument to the spread function. There may also be problems with class: directives, I'm realizing now as I write this. To be investigated sometime soon.

It may end up being that an additional argument to one of the spread-related helpers is the better thing to do in DOM mode as well. Or perhaps a separate function that updates a 'here are the attributes you need to mix in' object so that the class value (if present) includes the scoping class.

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

Successfully merging a pull request may close this issue.

1 participant