Nested function values not working in React#1144
Conversation
|
Just to be sure it's React-specific, I've tried adding the exact same pattern to the diff --git a/packages/jss-plugin-rule-value-function/src/plugin-nested.test.js b/packages/jss-plugin-rule-value-function/src/plugin-nested.test.js
index 4f1ee75..9c0f490 100644
--- a/packages/jss-plugin-rule-value-function/src/plugin-nested.test.js
+++ b/packages/jss-plugin-rule-value-function/src/plugin-nested.test.js
@@ -180,6 +180,12 @@ describe('jss-plugin-rule-value-function: plugin-nested', () => {
margin: () => '10px'
}
}
+ },
+ d: {
+ '&::before': {
+ content: '""',
+ height: () => '1px'
+ }
}
},
{link: true}
@@ -205,6 +211,11 @@ describe('jss-plugin-rule-value-function: plugin-nested', () => {
.c-id.a-id .b-id {
margin: 10px;
}
+ .d-id {}
+ .d-id::before {
+ content: "";
+ height: 1px;
+ }
`)
})
}) |
|
I think the problem originates in for (const key in meta.dynamicStyles) {
const name = `${key}-${meta.dynamicRuleCounter++}`
const rule = sheet.addRule(name, meta.dynamicStyles[key])because |
|
Yes, I the dynamic rules map doesn't know anything about the nested rules basically since they were added to the sheet.rules by plugins. The mistake that was made is that to assume that dynamicStyles are all dynamic styles we might have. |
|
Evtl. the right solution to this problem is to make the plugin that is responsible for dynamic styles to update that map. |
|
Do you recall what changed between the stable release and the v10 alpha to make it stop working? |
|
Yes, @HenriBeck optimized the sheet creation using just one sheet for both dynamic and static rules, which resulted in the need of knowing what dynamic rules belong to a specific component instance since you don't want to update all dynamic rules. Before that there was one shared static sheet and each instance had a separate sheet for dynamic rules. |
|
What we could do to detect these nested dynamic rules is to check how many rules have been added for the one call (through the |
yeah but also sounds like a hack |
|
What about adding a plugin in react-jss that reacts when a rule has been added? Using onProcessRule hook. |
But how would we get these rules from inside the component then? We either need to check first if we have nested rules, but also then figuring out their name feels hacky. |
Evtl. by turning on a flag before adding a rule and checking that flag in the hook. Once flag is true, every added rule is added during addRule() after that you have an aggregated list of rules and clean up the list and move on. That's just the idea in theory, I would have to try to see how this plays out. |
|
I will try to fix this on the weekend. |
|
This is fixed @kof |
* master: Use componentDidMount for WithStyles (#1157) Use mui like global ponyfill (#1153) HOC should not attach sheets until mount (#1149) v10.0.0-alpha.22 add react-dom dev dependenncy Fix SSR for Hooks based implementation (#1148) React-JSS id prop docs and improvements (#1147) v10.0.0-alpha.21 fix changelog add support hint to the changelog (#1145) Sheets management for css() (#1137) # Conflicts: # packages/react-jss/.size-snapshot.json
* Add failing tests to demonstrate nested dynamic rule issue * Fix nested dynamic styles
I've found that code like this:
Results in CSS like this:
That is, the result of the function value,
height: 1px;, is missing.This feature works in
react-jssv8.6.1 so seems like a regression somewhere between then and v10.This seems to be specific to
react-jssbecause the test in plugin-nested.test.js#L180 does something very similar and passes.This PR adds failing tests to
react-jss. I haven't figured out the cause of the issue yet.