nuxodin/ie11CustomProperties

Components which update classnames break the extra class rules

dzearing opened this issue · 12 comments

Consider a Toggle component in React:

import cx from 'classnames';
import * as classes from 'foo.css';

const Toggle = props => (
  <div className={cx(classes.root, props.checked && classes.checked) }>
...
  </div>
);

When the checked property mutates, classes are re-evaluated.

The polyfill works as expected on initial injection. Then the user clicks to toggle the checked state. The class attribute is repopulated with what cx(...) provides, thus trashing the classnames injected by the polyfill and dropping the resolved values.

I don't see how this can work with any classname altering components. They would have to toggle classes selectively, preserving current state. This might be just how React updates classes.

Are there any workarounds to making this work with React components which may alter classnames, other than "don't alter classnames?"

One suggestion that would resolve this: do not use class names for the extra classes. Instead, use data attributes:

Instead of:

<div class="somestuff iecp-u3">...</div>

This:

<div class="somestuff" data-iecp-u3>...</div>

And this in the generated rule:

.selector[data-iecp-u3] { ... }

That way if classes mutate, your rule doesn't disappear.

@nuxodin Are you actively working on this? Would like to collaborate if possible.

Thank you for your PR
The problem i see is, that attribute-selectors have more specificity.
I chose extra as little specificity as possible.
If i make this change, It can break existing pages...

What do you think?

I think this issue needs to be re-opened. This is a real blocker for usage in any web framework which mutates classes by replacing the class attribute content (e.g. React.)

I think the performance concern is a great catch. However I am not sure what an alternative solution would be to the issue. Is there a faster selector we could use that isn't className?

I'm not seeing an alternative. Could this be implemented as an optional behavior?

I have just experimented with "runtimeStyle" to set the styles.
It would be much faster, even than class attributes.
el.runtimeStyle[prop] = value;
To my astonishment even the tests fit.
Bud some things not working correct in the demo.html.
To be honest, I don't really know what I am doing anymore, I am reaching my mental limits :)
I don't have that much time left either.

Hey that's interesting! Do you have a branch with this in it?

https://github.com/nuxodin/ie11CustomProperties/tree/runtimeStyle-tests
This is great!
Performance is ~10x better!
(For custom properties not defined on the html-element, which are already fast)

can't wait to try, i'll pull it over to my test page this weekend!

4.0.0 should work now.

Will reopen if it does not work

Will reopen if it does not work