Fix(ReactVis): Stop Setting Refs On Stateless Components #867
Conversation
|
@mcnuttandrew and should I overwrite the previous buggy patch or push a new one? |
|
I like your fix! Is there a way to write a test about this so it doesn't happen again? (if not i'm not incredibly concerned, I think we'll get a similar bunch of people yelling about it) You should always cut new patches! Our versioning isn't super fancy or branded, so in my view it's better to have something that is actively versioned rather than try to fix buggy versions. Version 1.10.2 here we come!! |
|
@mcnuttandrew great, thanks - and sorry about breaking things! Spying on |
tests/setup.js
Outdated
| @@ -8,6 +8,9 @@ Object.keys(document.defaultView).forEach(function mapProperties(property) { | |||
| global[property] = document.defaultView[property]; | |||
| } | |||
| }); | |||
| console.error = (message) => { | |||
There was a problem hiding this comment.
this only works on React 16 for this specific bug, but it catches others
There was a problem hiding this comment.
It looks like you are failing lint? Also if this starts getting too rabbit hole-y, maybe push the console.error stuff into a seperate PR so this bug fix can go out?
There was a problem hiding this comment.
yea agreed, I'll remove this and ship it
* Fix(ReactVis): Ref On Stateless Component Error * cleanup * fix lint * add regression test * fix lint * remove regression test
* Fix(ReactVis): Ref On Stateless Component Error * cleanup * fix lint * add regression test * fix lint * remove regression test
00ffe61 tries to set the
refattribute on stateless components - which causes loads of console errors. This fix only sets therefattribute on components which were previously receiving it.#861