Radium: Remove unknown props on <div> tag

[kylecesmat]

The style-root component is receiving all props passed to it. We should remove unused props as future versions of React React v15.2.0 will throw warning messages.

Warning: Unknown prop `_radiumDidResolveStyles` on <div> tag. Remove this prop from the element. For details, see https://fb.me/react-unknown-prop
    in div (created by StyleRoot)
    in StyleRoot (created by App)
    in App

Line in question - https://github.com/FormidableLabs/radium/blob/master/src/components/style-root.js#L35

Related to this issue - https://github.com/react-bootstrap/react-bootstrap/issues/1994#issuecomment-229699699

[didierfranc]

Same issue with my project after React 15.2.0 update

[nbrady-techempower]

Just came here to post this, thanks for taking care of it so fast @kylecesmat !

[alexlande]

Quote from the related PR:

This doesn't do the trick, unfortunately. This prop is applied with our _cloneElement wrapper, so we can't handle it in the render method. Furthermore, this happens with _all_ elements processed by Radium, not just StyleRoot. Taking discussion back to the open ticket to figure this out one.

A short-term solution would be to make this prop a data attribute to silence the warning. This would render the attribute to the DOM, which is annoying, but that will happen anyway in React 16. Longer term... I'm not sure. We might think about a different mechanism for this behavior.

Edit: Tried this out, and it is extremely noisy rendering a data attribute on every element in an app. Worst case we can do so, but we should try not to.

/cc @ianobermiller

[awkaiser]

Since Radium is using Babel, I was thinking about implementing a fix for this utilizing an ES2015 WeakMap to track which elements had already been processed (sidestepping need for DOM attributes), but that appears to not be allowed by the project configuration. @alexlande, is that an allowance to consider now or would that open a can of worms?

[alexlande]

@awkaiser: I like that idea, but we don't currently require any polyfills (beyond those required by React itself), and I'd like to keep it that way if we can.

[ianobermiller]

weakmap isn't an option because it cannot be polyfilled correctly. We'd have to manually keep track and somehow cleanup on componentWillUnmount.

[tquetano-r7]

Alright this is simplistic, but bear with me.

Currently the only thing _radiumDidResolveStyles does is validate have the styles already been resolved ... do we re-clone the element with newProps or just return the already-cloned element. If we change _radiumDidResolveStyles to a data attribute (something like data-radium-did-resolve-styles), the warning goes away and it works the exact same. It adds a small amount of clutter to the DOM (which can be aided by a less-verbose name), but other than that it solves the problem with minimal code change.

I didn't want to submit my PR if people were gonna blow up at the concept alone, so let me know if this is okay and if so I'll submit straightaway.

[alexlande]

@tquetano-r7: yeah, I floated that previously: https://github.com/FormidableLabs/radium/issues/759#issuecomment-230531343

I don't particularly mind it, and if React 16 came out tomorrow and this was a breaking change we would probably do it. I'm hoping we can land on a different solution that doesn't result in a ton of extra attributes in the DOM, but worst case we'll use data attributes.

[tquetano-r7]

ah sorry @alexlande i missed that comment. fair enough.

[awkaiser]

How about data-radium-drs="1" (did resolve styles), data-radium-r (resolved), or even data-radium="1" if this is the only data attribute use for Radium right now? I hear you on DOM noise and don't like the idea any more than @alexlande but, in the interest of clearing this warning, why not go with a data attribute for now and update with a better solution later, rather than holding this particular issue open?

[alexlande]

Using a data- attribute has an actual effect (although a small one) on users, while leaving the error in place has no effect except that it's a little annoying in development mode.

That said, based on the interest in this issue, it seems that the community feels more strongly about having to see the deprecation warning than I do, so maybe it's best that we add a data attribute for now and revisit after.

[alexlande]

This is fixed in 0.18.0

[doctyper]

@alexlande I'm still seeing this issue using <StyleRoot> with a radiumConfig parameter:

<StyleRoot radiumConfig={{
    userAgent: "..."
>
    <div>Foo</div>
</StyleRoot>

Warning: Unknown prop `radiumConfig` on <div> tag. Remove this prop from the element. For details, see https://fb.me/react-unknown-prop
    in div (created by StyleRoot)
    in StyleRoot (created by ApplicationContext)
    in Perf (created by ApplicationContext)
    in ApplicationContext (created by RouterContext)
    in RouterContext (created by Router)
    in Router (created by Constructor)
    in Constructor

[alexlande]

Ah, thanks @doctyper. That one's an easy fix, at least. PR incoming!

[alexlande]

That's resolved in #787, for any digital archaeologists looking at this thread in the future.

[russellr922]

@alexlande, I'm still getting this Warning with the latest version of Radium:

Warning: Unknown prop 'radiumConfig' on <div> tag. Remove this prop from the element. For details, see https://fb.me/react-unknown-prop

Any ideas?

"radium": "0.18.2"

[romshiri]

Me too. Still getting this warning.

[alexlande]

Reopening, I'll try to repro this.

[russellr922]

@alexlande, important to note, I experienced this when using a <div> with a radiumConfig prop. This issue does not occur when using the <StyleRoot>. I'm not sure if @romshiri is experiencing the same, but if so I think this issue can remain closed in favor of properly using the <StyleRoot> component.

[alexlande]

Ah, I see. Thanks for the update @russellr86. @romshiri, can you confirm that you're passing radiumConfig to the StyleRoot component?

[romshiri]

My bad, I was getting another warning, and it was my fault of using the StyleRoot in appropriately anyway.

[alexlande]

No problem, thanks for checking 👍