-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Improving the scope attributes #570
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
Comments
Thanks — this is useful feedback, I agree about the verbosity. It is a bit ugly. The main reason for using hashes is that it guarantees consistency — if you render a component on the server and again in the client, the hashes will always match as long as it's the same source. This means the styles are always correct and never out of date. We could use names instead if we were prepared to loosen that guarantee, but conflicts would be a problem — the compiler only considers components in isolation, it doesn't know that you have two Using class names instead of attributes could certainly work, even if classes are dynamic. It would make the markup slightly more verbose... <!-- this... -->
<div class='svelte-123456'></div>
<!-- instead of this -->
<div svelte-123456></div> ...but if there's a performance gain to be had then it's probably worth it. Do you have any numbers you can share? |
Ah, I see. I found
I'm not sure if using querySelectorAll() on jsperf accurately reflects the way browsers work while styling elements, but I put together this test: https://jsperf.com/queryselectorall-attr-vs-class-selector When I try it with Firefox and Safari I'm actually not seeing much of a difference between attributes and classes. Chrome does show a more significant advantage when selecting only .class instead of [attr], but that along with the results of the other browsers is still not nearly as much as I had expected. I suppose browsers today have been optimised to the point where it might only come into play when there are thousands or even tens of thousands of elements involved. http://stevesouders.com/efws/css-selectors/csscreate.php which I found through a comment of yours (which also mentions the performance concerns) can be used to test those scenarios, and do show a greater difference when I test, but I'm not sure if that's reason enough for this change. |
The selector speed issue was relevant about 10 years ago in IE6 and IE7 era browsers. I think currently the selector speed is a non-issue. If the page/app is slow it's because of other issues. There might be some issues in slower and cheaper smart phones but I've not seen these kinds of problems in years. I think the attribute as a selector is considered as a bad practice. I'd like to see the attributes moved to class names. |
@Glenf if not for performance reasons, why are attribute selectors bad practice? |
@Rich-Harris Actually I think I'm mixing things up here. Using attributes as selectors in CSS is something that's not a good practice. Partly because of css specificity and partly because of the selector speed. In this case since attribute selector is something that actually specifies the style even more the first part is kind of irrelevant. The second part, I'm not sure if speed is an issue. |
The speed of various selectors were discussed more recently than the times of IE6/7 for sure - I recall seeing articles between 2011-2014 - but having looked further into it now it seems safe to say that the performance difference between the selector types are trivial in the browsers of today. My main interest with this issue was to see whether the hashes could be turned into something more useful/less intrusive to human eyes. Since that may not be possible without adverse consequences, it's okay for me if we close this issue, though I'll let you decide if there is something left to discuss. |
Specificity isn't really an issue, considering attribute selectors and class selectors have the same specificity. That means you can replace an attribute selector by a class selector or vice versa without impacting specificity.
The notion that attribute based selectors are slower than class based selectors used to be wide-spread among CSS experts, but I haven't seen much data actually supporting it besides Steve Souders work on CSS selectors from 2009. A lot has changed since then, and today it seems we can pretty much ignore performance differences between selectors.
The main reason I avoided the use of attribute based selectors because they weren't supporter in IE < 7, Opera < 9 and Safari < 3, whereas class selectors had pretty much universal support. However, as those browsers aren't relevant anymore, there really is no good reason anymore to avoid attribute based selectors IMO. So I'm OK with using I do have an issue with use of randomized hash values, though. Besides being rather ugly and non-semantic, these attributes also prevent efficient component-specific global theming. See #379 (comment) for details.
If you add a check to validate whether the Anyway, each time you re-build your code, the value of the hash changes, which means that you can't create a global theme to override the component-specific styles of a particular component without impacting other components. This would be fixed if we could just set an (optional) For me, adding this feature would be a major improvement! |
The point is that they're not randomized, they're a hash. They guarantee that whenever you compile your code the hash is the exact same.
I think your issue with global theming comes down to this, but I feel like this solution already works: <div class="my-component"></div> In response to your comment on #379, it seems in that case you would simply go the route of not including "default" styles in your components. If you want to style them with a global theme, then use a global theme. |
What if I modify my code? I mean... I don't want to generate my CSS themes every time I change a line in my components... How exactly is the hash generated?
In reality, my component would look something like this : <div svelte-1535408840 class="my-component">
<div class="button">Press me</div>
</div> /* generated component CSS */
[svelte-1535408840] .button {
background : #ccc;
}
/* hand-written global theme CSS */
.my-component .button {
background : #ccc;
} This bothers me for several reasons :
It would be infinitely better to be able to do something like this : <div svelte-component>
<div class="button">Press me</div>
</div> /* generated component CSS */
.svelte-component .button {
background : #ccc;
}
/* hand-written global theme CSS */
.svelte-component .button {
background : #ccc;
}
The use case I have in mind for Svelte is a UI component framework built on top of a CSS framework, that itself can be used as a generic foundation for any JS project. Without being able to use three layers of CSS - as described in #379 - this simply won't work. |
Another data point, suggesting that appending classes may be better than adding an attribute: |
I think we can close this. The 'random' codes are needed for encapsulation of styles - and we have another issue (#1118) dedicated to the task of switching from attributes to classes. If someone disagrees, this can be reopened. |
Using base26 and the component name are both interesting ideas! Reopening. |
See #1192 — uses classes, base 36, and (possibly, not sure yet if we want this) |
Just released 1.57 which switches to classes and base36. I decided against defaulting to using the component name, since it reduces compressability, but I think it would be worth making it configurable. From #1192 (comment): const counts = {};
const incr = id => {
if (!counts[id]) counts[id] = 0;
return counts[id] += 1;
};
const { code, css, map } = svelte.compile(source, {
class: ({ name = 'svelte', filename, hash }) => `${name}-${incr(name)}`
}); Will leave this open until we get round to implementing that. |
Somehow I missed this issue/thread. I'll just chime in on one thing which has to do with semantics. (It's already been said, but I think it bears repeating, perhaps ad nauseam 😅.) I thought one of the points of semantics on the web was readability; base36 hashes not being such? Maybe a fools errand, but this might also go to a component marketplace. Dropping something in place and having a never-changing semantic namespace that someone knows by name. FYI, hashes might still be the right thing, but this is definitely a problem that needs to be thought out within the larger context of dropping components into random and sundry places. |
@arxpoetica Your existing semantic classes etc are preserved for that reason. The additional scoping class has one job - avoiding clashes - and so it kind of has to be based on a hash, doubly so if we're talking about a component marketplace |
Huh. No need to explain; I won't beat a dead horse and I totally trust your mentality on this, but I can't put two and two together. 😄 |
p.s. I've come around to Rich's thinking on this... |
This comment has been minimized.
This comment has been minimized.
The |
I appreciate the attention that's gone into making Svelte output readable code. It makes it easier to debug and understand what is happening at a glance. However, one part which I feel stands in contrast to this is the
svelte-[hash]
attributes. They're pretty verbose, making the rendered DOM harder to inspect and consume visually, while offering no human-friendly clue as to where they belong.Under the assumption that the purpose of the hash is only to serve as a unique identifier for a component, I believe these concerns could be resolved by replacing the hash with the name of a component instead. In the event of two components having the same name, they can be deconflicted by appending a number as is done in the script.
Having made a few modifications to the compiler based on this idea, I compiled the templates for
svelte.technology
. Here's a compact example showing before and after:https://gist.github.com/Zirro/79e016d2e84ca98976c2fa5a5ab7b9b9
https://gist.github.com/Zirro/708b96d3046b72af98971477fa9282ea
You'll note that there's another change as well - it's using classes instead. I decided to try this for two reasons - one of fairly little importance, which is that Firefox's
view-source
renders attributes in bold black letters, making them quite a distraction.More importantly is that selecting elements by attributes is measurably slower than doing so with classes. I figured that a class can do the job just as well, provided that it uses a name with virtually no risk of clashing with ones that site developers would add.
For this, I conveniently came across • in the
<title>
of the Svelte homepage. I believe it strikes a good balance, showing that the class is special without being a distraction. It can also be set together with other classes in a single.className
instead of a separatesetAttribute()
.I suppose this is two suggestions in one. The hash -> name and attribute -> class changes can both be discussed separately.
What do you think? Is there something which I've overlooked that would make this impractical?
The text was updated successfully, but these errors were encountered: