Fix for missing dynamic rules #1642
Conversation
| @@ -114,7 +114,10 @@ export default class RuleList { | |||
| remove(rule) { | |||
| this.unregister(rule) | |||
| delete this.raw[rule.key] | |||
| this.index.splice(this.index.indexOf(rule), 1) | |||
There was a problem hiding this comment.
In fact, it should work with indexOf, while only adding the -1 check, right? If help is needed, I can try myself. What else is needed to merge the PR? Some tests?
There was a problem hiding this comment.
I lack the greater context to effectively debug the reason why is 'find by reference' not working.
In fact, it should work with indexOf, while only adding the -1 check, right?
If we checked only for -1 it wouldn't be removing old styles
There was a problem hiding this comment.
why is by reference not working?
I am not quite sure, but I think it has to do with the following:
When some dynamic rules are updated because the data might change, those new updated rules are not stored in the useStyles hook. So, finally when removeDynamicRules happens, we remove the old rule references, instead of the new ones. I think this fix handles that correctly, but some new test cases should be added. Would be very nice if a new version would be released soon, since JSS is still widely used.
|
Hi @Bundas, just wanted to know if there are any updates about this. Can I help in some way? Cheers |
Corresponding Issue(s):
What Would You Like to Add/Fix?
#1635
Todo
Expectations on Changes
Changelog