CodeMirror onChange event appears to have an outdated store
Kycermann opened this issue · 7 comments
I have a function component that includes a Controlled CodeMirror component.
function CodeArea({store}) {
... console logging ...
return (
<CodeMirror // Controlled
onChange={() => {
... console logging ...
}}
/>
);
}
Just before returning:
console.log("Render", store.openTabs.map(data => data.id), store.currentTabIndex);
First line of CodeMirror onChange
:
console.log("onChange", store.openTabs.map(data => data.id), store.currentTabIndex);
Render Array [ "4" ] 0
Render Array [ "4" ] 0
__RR__.internals.store.openTabs[0].id = "2"
"2"
Render Array [ "2" ] 0
onChange Array [ "4" ] 0 // Why is this "4" and not "2"?
Render Array [ "2" ] 0
onChange Array [ "2" ] 0
Render Array [ "2" ] 0
I added extra code in onChange
to try to isolate the issue:
console.log("Start onChange log:");
console.log("onChange", store.openTabs.map(data => data.id), store.currentTabIndex);
console.log("onChange", window.__RR__.internals.store.openTabs.map(data => data.id), window.__RR__.internals.store.currentTabIndex);
console.log("End onChange log.");
Render Array [ "4" ] 0
__RR__.internals.store.openTabs[0].id = "2"
"2"
Render Array [ "2" ] 0
Start onChange log:
onChange Array [ "4" ] 0 // these two lines should
onChange Array [ "2" ] 0 // have identical outputs
End onChange log.
What version are you using?
The below works fine, does that minimally represent what you're trying to do?
import React from 'react';
import { Controlled as CodeMirror } from 'react-codemirror2';
import 'codemirror/lib/codemirror.css';
import 'codemirror/theme/material.css';
import 'codemirror/mode/javascript/javascript';
import { collect, initStore } from 'react-recollect';
import './styles.css';
initStore({
tabIndex: 0,
tabs: [
{
name: 'Tab one',
code: `const tabOne = 'foo';`
},
{
name: 'Tab two',
code: `const tabTwo = 'bar';`
}
]
});
const App = ({ store }) => {
console.log('render store:', store);
const currentTab = store.tabs[store.tabIndex];
return (
<React.Fragment>
{store.tabs.map((tab, i) => (
<button
style={{
background: i === store.tabIndex ? '#4fc3f7' : '#eee'
}}
onClick={() => {
store.tabIndex = i;
}}
>
{tab.name}
</button>
))}
<h1>{currentTab.name}</h1>
<CodeMirror
options={{
mode: 'javascript',
theme: 'material'
}}
value={currentTab.code}
onBeforeChange={(editor, data, value) => {
console.log('onBeforeChange value:', value);
currentTab.code = value;
}}
onChange={(editor, data, value) => {
console.log('onChange value:', value);
}}
/>
</React.Fragment>
);
};
export default collect(App);
Could you perhaps fork that CodeSandbox to replicate the issue you're seeing?
BTW, in CodeSandbox, because the app runs in an iFrame, you need to select the frame ending in .app
in the console to get access to __RR__
:
BTW@, when writing code blocks in GitHub you can do "js" or "
jsx" to get better syntax highlighting - I updated your comment.
I've tested it with using 5.0.0 and 5.1.0.
I've replicated the problem to an extent. The difference is that in my code the discrepancy occurs any time a tab is switched but in the forked CodeSandbox there's only a discrepancy the first time you switch tabs.
Thanks for that. I see what's happening now. It's all behaving as expected, but I can see how it must be causing some head-scratching...
In the example, you're not reading store.testActiveTabs[0]
when rendering (it has no effect on how the component renders) so the component doesn't get subscribed to changes to the value and doesn't re-render when you change it.
If you use it during the render, the problem goes away. Something like this:
const currentTabIndex = store.testActiveTabs.includes('1') ? 1 : 0;
const currentTab = store.tabs[currentTabIndex];
Or you could use the useProps
function, which lets you subscribe to changes in a property that you don't use to render - this is generally not required though.
There's a few other things going on, complicating the matter. First, store.tabIndex
is changing and that does trigger a re-render. But the lines after that don't trigger a re-render (if you flipped the order, the problem goes away - obviously that's not the fix though).
Also, note that reading store.testActiveTabs
(e.g. logging it to the console) doesn't subscribe a component to changes in every item in the array (by design). Logging store.testActiveTabs[0]
would subscribe to changes in the first index. Logging store.testActiveTabs.join()
or .map()
or anything that reads every item in the array will subscribe the component to changes in every item in the array.
Also, it seems that <CodeMirror>
is calling the onChange
callback during the render cycle of the component, not after. And since you're reading testActiveTabs.map()
in the onChange
to log it to the console, after that first change the component becomes subscribed to changes in that [0]
slot.
So normally, this problem would just be a matter of "why isn't my component updating?" and the answer would be "because you're not subscribed to the prop that you're changing". But CodeMirror reading what you're logging during the render cycle makes it not at all obvious what's going on.
Thank you for the information!
I can see that with useProps
the problem goes away- because Recollect knows about store.testActiveTabs
. I understand that not being subscribed to changes in store.testActiveTabs
does not cause re-renders- but I'm confused as to why the old value is provided when it is accessed during a render cycle in onChange
.
In my case the props are already being used, and even with useProps([store.openTabs])
the problem persists. I'm currently using the global store
object (and collect
) for this component- or wrapping the onChange
code in setTimeout
also fixes the issue. I can try to reproduce this in CodeSandbox if that would be useful.
How comes the values in a store
prop can ever be different from the global store
?
My pleasure, it's good to explain it and to work out which aspects are confusing to I can cover it better in the readme.
useProps([store.openTabs])
will subscribe you to changes in the array, but not to it's children. useProps([...store.openTabs])
would read every child.
Can you confirm that in the logs, you see the component reading from the prop? Make sure it's the actual thing you're changing (e.g. not the parent array, if you're changing an item in the array). Like this:
Also, when troubleshooting, looking at __RR__.internals.store
might just make things harder, since this is going to reveal all sorts of internal logic and make it harder to see simple problems like the component simply not being re-rendered.
In not, then yeah if you could replicate a component reading a prop but not re-rendering when it changes that would be great. Ideally only using props.store
.
Here's some more detail...
Once a component has been rendered with the store passed as a prop, that store is never mutated. And during the render cycle, any read from the store reads from this prop (as you would expect). This is so in shouldComponentUpdate(prevProps)
, you can have access to the 'previous' store, but also because React uses this internally when optimising.
Outside the render cycle though, all reads from the store are routed to the most recent version of an object (what you would consider normal behaviour for a mutable object). This is so you can do this in, say, an onClick
for a button:
props.store.tasks = [];
props.store.push(/* blah */);
In that case, if you read from the store as it was passed to the component originally, line two wouldn't see the new .tasks
array, since that didn't exist when the component last rendered.
Hence, during a render cycle the stores are in an interesting state, where it is possible to see that they are different. At all other times, you don't need to worry about the fact that there are multiple versions of the store. This is especially complicated if you trigger two updates, only one of which causes a re-render, and compare the global store and the store passed as props.
This is a step-by-step for that CodeSandbox demo:
- The component renders and is subscribed to changes in
tabIndex
, but nottestActiveTabs[0]
. - In
onClick
for the button,tabIndex
is changed on line 35. - At this exact point in time,
tabIndex
has been updated, but the line that setstestActiveTabs[0]
hasn't run yet. - Recollect triggers an update on the component, because it subscribes to
tabIndex
, passing the updated version of the store, wheretabIndex
is1
buttextActiveTab[0]
is still'0'
. The 'update' is performed by setting the state inside thecollect
higher order component, allowing React to be clever and batch updates. - React is going to continue processing the contents of that
onClick
to see what else happens before actually kicking off a render cycle. - A microsecond later, when
onClick
is finished, andtestActiveTabs[0] = blah
has executed, Recollect updates the store (but not the version of the store that's already been provided to the component in step 4 - the store passed to a component is never mutated). - Because nothing is subscribed to
testActiveTabs[0]
, Recollect doesn't queue any components to update. (If the component was subscribed to this property, Recollect would queue a second update and the first from step 4 would be skipped.) - Now React executes the render. At this point, the component has the store from step 4, where
tabIndex
is1
buttestActiveTab[0]
is still'0'
. but the 'global' store has the data as it was updated in step 6.
And the rest is probably a bit more obvious. The CodeMirror onChange
runs, reads testActiveTab[0]
so the component is subscribed to changes and behaves as expected.
You can see the above story in the logs. Note that there's only one update (this actually means 'queue update')
However, if the component was subscribed to testActiveTabs[0]
, the logs would look like this - note the second update before the actual render starts:
Any news on this?
FYI two new debugging helpers were added in 5.2 https://github.com/davidgilbertson/react-recollect#window__rr__
Let me know if you'd like me to keep this open.
My bad, I was on an inactive streak. That all makes sense though, thank you very much!
If I want to subscribe to changes to an array and its children, I do useProps([store.array, ...store.array])
and this works.