Recomputes css hash id when unused selectors are present #4326
Conversation
This is an attempt to fix sveltejs#4313 which demonstrates that incorrect css can be produced when two components have the same style rules and the first component to be included has unused selectors.
| const unused_rules = this.children.reduce(collectIf(rule => !rule.is_used(component.compile_options.dev)), []); | ||
| if (unused_rules.length > 0) { | ||
| const new_hash = hash(used_rules.reduce((acc, rule: Rule) => { | ||
| for (let i = rule.node.start; i < rule.node.end; i++) { |
There was a problem hiding this comment.
is there an easier way to get the raw text of a node?
| } | ||
| return acc; | ||
| }, '')); | ||
| this.id = `svelte-${new_hash}`; |
There was a problem hiding this comment.
to me, reassigning the stylesheet id here seems clunky even though it does fix the reported bug. i would've preferred to set the id accounting for the unused selectors in the Stylesheet constructor but, at this stage, Rule.is_used() only returns false. it isn't until after the html Fragment is created that the dom nodes are paired with the css selectors and Rule.is_used() can accurately report which are unused. any suggestions on how to improve this flow would be appreciated.
|
The id being recomputed doesn't just feel clunky, I think it indicates that there's a situation this isn't handling. If I comment out the first place where A proper solution for this sounds like it will probably involve multiple sweeps through the template AST: one to note which elements need the scoping class added to them and to determine which selectors are needed, and another to apply the scoping class whose id will depend on which selectors were kept. |
|
or another solution is to include the template into the computation of the hash too. the template indirectly have the information of used/unused selectors, which is part of the recomputing of hash. |
|
Using the template in the hash would probably be the simplest. That's how it used to work before #1091. I'm not completely clear on the reasoning for the switch, and unfortunately the gitter chat room has been deleted so we can't see what i18n thing it was also intended to help with. |
This is an attempt to fix #4313 which demonstrates that incorrect css can be produced when two components have the same style rules and the first component to be included has unused selectors.