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

feat unused css selector that understands string concatenation #3825

Merged

Conversation

tanhauhau
Copy link
Member

Fix #1698

@tanhauhau
Copy link
Member Author

on a separate note,

while working on this case:

<div class="foo {a ? 'bar' : 'baz'}">

i realised there's many way to implement it:

(from most preferred way to least preferred) or
(from easiest to optimised during compile time to hardest to optimise)

<h1
     class="foo"
     class:bar="{a}"
     class:baz="{!a}" />
<div class="foo {a ? 'bar' : 'baz'}" />
<div class={`foo ${a ? 'bar' : 'baz'}`} />
// or
<div class={'foo ' + (a ? 'bar' : 'baz')} />

however because of having writing react app, im more inclined to implement it the 3rd way.

i wonder maybe we should have give warnings and guidance to user who write in the 3rd way, and guide them to write it like the 1st way, with svelte's class directive

@Rich-Harris what do you think?

@Rich-Harris
Copy link
Member

Oh man, this is heroic!

I tend to lean away from being overly didactic about writing code in a more compiler-friendly style (except in cases where the difference is quite significant), because I think there's a danger that we start to unconsciously favour patterns that are helper for us as implementers, but that are more work for component authors. So I think this PR is great as-is, and we shouldn't worry too much about people who write in the second and third styles (I'm personally quite fond of the second 😀 )

@Rich-Harris Rich-Harris merged commit 33ebcfb into sveltejs:master Nov 6, 2019
@tanhauhau tanhauhau deleted the tanhauhau/unused-css-string-concat branch November 6, 2019 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Smarter CSS encapsulation with concatenated classes
2 participants