Chore(ReactVis): Remove String Refs

[benshope]

I just ran into #736 and this will probably fix it

[radumiron]

Wow, I'm looking forward to see the build pass on this one!

[jckr]

Thanks @benshope for taking this on!

[benshope]

@radumiron yea I pushed this branch before checking the tests, I'll fix those!
@jckr 👍

[benshope]

@jckr should I use normal methods instead of arrow functions for perf reasons?

[mcnuttandrew]

@jckr should I use normal methods instead of arrow functions for perf reasons?

Using arrow function vs explicit functions shouldn't change anything right? It's all passed through babel and transpiled into straight old fashioned js

[benshope]

@mcnuttandrew yea I guess it's probably fine here. I had read a few headlines about arrow functions in render being an issue https://medium.freecodecamp.org/why-arrow-functions-and-bind-in-reacts-render-are-problematic-f1c08b060e36 and figured that would apply here - but I guess that article is saying they're mostly problematic higher in the tree when trying to prevent unnecessary refreshes. Good to ship this if you're cool with it!

[mcnuttandrew]

Yea seems good! This feels like this might be a kinda significant change, so maybe do a minor bump

[ericsoco]

@benshope the issue is not with arrow functions in render, it's any function declared in within render. The issue is that React compares current props with previous, sees that the function created last pass !== the fn created this pass, and therefore rerenders the component tree from there down.

Doesn't matter if it's an arrow func or pre-ES6 inline func, just that it's declared within render.

Ideally, callbacks-as-props are declared once, as a member of the React class or outside of the class def, and params are passed to it. This is not always possible, i.e. when context from within render needs to be passed in and can't be pulled e.g. off of this.props. In that case, you just accept the hopefully-minor perf hit.