-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Set style attributes directly instead of using cssText for inline styles #455
Comments
Just wanted to note that the ever lovely CSSStyleDeclaration has a I don't know how it benchmarks in comparison to setting the properties directly though, but I'd hope as well as, if not better. |
The most performent thing to do would be to know at compile time what CSS styles we're setting, however we can't quite statically analyze the value of a <div style:top="{{top}}" style:position="relative"/> which then would tell us what styles we're setting at compile time. I might be way off, but that's just my idea. |
Ooh, that might be a good idea. My initial thought was that we'd have to use an object of keys+values for styles, as in React. But I do like the idea of knowing at compile time what all the CSS properties are. I think both mechanisms ( |
Oooh that is interesting. Just for my own understanding, the only way the style attribute would not be able to be statically analyzed is if a style property name itself is set dynamically, right? Like this: Not possible to analyze
Possible to analyze
I guess it is possible I know for me personally, I am never setting any style properties dynamically. I have done things like this:
That also has the property itself available at build time though. |
Ractive supports individual style properties as attributes e.g. It may be worth noting that having |
@ccampbell But you're still setting it dynamically, that ternary is ran at runtime and as well the value of width could be anything. So I've been thinking about the syntax and I'm definitely wondering if along with defining styles we could mix in the shorthand attributes implemented with #384 (linking to PR since there's no documentation I found yet.) <div :style:top :style:left/>
<script>
export default {
data() {
return { top: '5px', left: '15px' }
}
}
</script> |
Ah okay I gotcha. Technically that could be compiled to something like this if that string was parsed also:
but probably that is a lot more effort than it is worth. I like the EDIT: Maybe you could use |
Yeah, the I'd be interested in exploring whether we can infer key-value pairs from an attribute like this: <div style='top: {{top}}px; left: {{left}}px;'></div> As others have pointed out, you could definitely do something wacky that would break expectations, but I think in 99% of cases you could correctly translate that to... <div style:top='{{top}}px' style:left='{{left}}px'></div> ...i.e., read a key, check it's a valid CSS key, then treat everything up to the next I'd probably vote against doing the same thing with class because a) there's the browser support thing @evs-chris mentions, and b) classes don't typically change with the same frequency, unlike styles which could easily change 60 times a second.
Maybe (and I'm just throwing this out there) we stipulate that |
@Rich-Harris The issue is we don't know if the value is an integer or string, so it would break if someone passes '12pt'. |
Right, I'm saying we stipulate that if you choose the |
I definitely think we shouldn't do that for users, but how much code would it be to add it to the runtime?
doesn't seem too bad, especially if we threw it into a helper function. Also we'd only need to add it if someone uses the |
@PaulBGD @Rich-Harris just to clarify you are talking about using the proposed shorthand of |
Regarding the distinction between having mustaches and not, the approach I've taken for explaining it is that things that are inherently strings, like the values in an inline style, attribute values, and interpolators amongst text nodes, require the mustaches. Things that are inherently values, like variable bindings/mappings, references inside a condition (e.g. it's For classes, yeah, individual/independent access doesn't really make much sense until you get into decorators. |
I'm just talking generally, I don't think the shorthand should act any differently. |
The issue is if you do |
@ccampbell Could be, but I think the reasoning is that if you have an invalid number for a CSS property, most browsers won't show the value in the inspect panel. |
This convo has persuaded me that we definitely shouldn't try to guess what units the user meant, and should always require that it be explicit — too much opportunity for confusion. Forget I mentioned it 😀
We can always add dev mode checks that tell the user if the value is invalid CSS |
Regarding key-value pair inference, it seems to me that having a parser specifically for reading inline styles would be fairly reasonable, as parsing out style pairs isn't too terrible if you aren't concerned with validating the values. I think you could safely allow conditional blocks and more complex value mustaches that way. The failure mode on the parser when running across e.g. |
I just don't see the need to write a complex CSS parser if you can just specify what styles you're using, which could even support the existing shorthand to make it even easier. |
@PaulBGD it's about making it easy for people to get the benefits of directly setting styles without having to learn new syntax. I'm probably still going to do
I reckon we'd allow one or the other for any given element, but not both. |
That was what I was originally thinking. Also cool with either/or. How do you think conditional styles would work? I have some things right now there are like <div style="{{ playing ? `transform: translateX(${getWidth(time)}px);` : '' }}"> I think that would be the equivalent of this in the new proposed syntax: <div style:transform="{{ playing ? `translateX(${getWidth(time)}px);` : '' }}"> I’m just not 100% sure how that would be handled. If the attribute is empty probably it should be reset to |
For most (all?) style properties I think setting it to the empty string resets it (and allows it to inherit styles: https://jsfiddle.net/wbLchk0q/) (Edit: |
But what about cases such as this one, which was suggested up above:
|
@PaulBGD combination of dev mode warning and compiler option should be enough, I reckon. Seems unfair to penalise the 99% just in case the 1% do something bonkers! |
As of 1.36, style attributes will be optimized to <!-- setProperty('color', state.color) -->
<div style='color: {{color}}'></div>
<!-- setProperty('transform', "translate(" + state.x + "px," + state.y + "px)") -->
<div style='transform: translate({{x}}px,{{y}}px)'></div>
<!-- bails out -->
<div style='{{key}}: {{value}}'></div>
<div style='{{style}}'></div> Unchanged (or static) properties are ignored when the DOM updates. I haven't implemented |
I would like to pass an object to style property. Like this: <script>
const myElementStyle = {
width: 100,
height: 100,
display: 'block',
color: 'red',
}
</script>
<div style={myElementStyle}></div> Instead of: <script>
const width = 100,
const height = 100,
const display = 'block',
const color = 'red',
</script>
<div style="width: {width}px; height: {height}px; display: {display}; color: {color};"></div> Or instead of: <script>
const width = 100,
const height = 100,
const display = 'block',
const color = 'red',
const myElementStyle = `width: ${width}px; height: ${height}px; display: ${display}; color: ${color};`
</script>
<div style={myElementStyle}></div> Yes, I know I can make use of <style> css, but if we need to change these properties dynamically? I would like to do the following: <script>
let pos ={
x: 0,
y: 0
}
$: myElementStyle = {
top: pos.y,
left: pos.x
}
function handleMousemove(e) {
pos.x = e.clientX
pos.y = e.clientY
}
</script>
<style>
.viewport {
width: 400px;
height: 200px;
border: 1px solid gray;
position: relative;
}
.viewport > div {
width: 50px;
height: 50px;
position: absolute;
}
</style>
<div class="viewport">
<div
on:mousemove={handleMousemove}
style={myElementStyle}
>
{pos.x} x {pos.y}
</div>
</div> There is an approach relative to this? UPDATECurrently I've made a helper to format a javascript object to style format: // utils.js
export function formatStyle(style) {
const appends = {
width: 'px',
height: 'px',
maxWidth: 'px',
maxHeight: 'px',
top: 'px',
bottom: 'px',
left: 'px',
right: 'px',
fontSize: 'px',
}
let result = ''
for (let property in style) {
result += `${property}: ${style[property]}${appends[property] || ''};`
}
return result
} <!-- My component -->
<script>
import { formatStyle } from './utils.js'
const myElementStyle = formatStyle({
width: 100,
height: 100,
display: 'block',
})
</script>
<div style={myElementStyle}>...</div> |
To save future wanderer time: the syntax discussed here doesn't work in 2020. Only standard HTML |
|
Sure. But what definitively doesn't work (and I wasted enough time on it...) is the |
I came across this issue from Google and wanted to mention that every individual style set causes a reflow. Using |
I am working on an app where there is a lot of motion and positions that are updated dynamically based on the state.
I threw together a very crude demo here:
https://svelte.technology/repl?version=1.13.2&gist=ac946b7b6802e16d0e963d3043c5d1d2
Anyway, I noticed thate the generated code uses
element.style.cssText
. I didn’t really think much of it, but I thought I would throw together a jsperf test to compare it to setting the styles attributes directly. (I found a few existing jsperf tests, but they were just setting the same property/value over and over, so I didn’t really trust the results)https://jsperf.com/csstext-vs-style-moving-element
It looks like
cssText
is approximately 15-30% slower in Chrome (in Safari it is around 50% slower).Also I threw a test in there using
cssText
with unchanged styles and that makes the performance quite a bit worse. I am not 100% sure about what svelte is doing, but I believe it is rendering out the full style attribute string every time it re-renders an element.I think it would be nice to update svelte so that it sets style properties directly. Not only would that be a bit faster, but it would allow the generated code to only update the styles that have changed vs. resetting the entire style attribute string each time.
I am sure that is quite a bit of work, but perhaps this could be considered a performance optimization for the future!
The text was updated successfully, but these errors were encountered: