Immutable mode for nestedProperty

[vdh]

Related to the immutable mode idea: #2389

While exploring an immutable mode solution for react-plotly-js-editor, I noticed a dependency upon Plotly's nestedProperty.

What would be the most ideal way to make an immutable alternative for nestedProperty?

In a generic scenario the dependency is:

const prop = nestedProperty(container, attr);
prop.set(value); // container is mutated

And an alternative immutable API in nestedProperty could do this instead:

const prop = nestedProperty(container, attr);
const newContainer = prop.setImmutable(value);

Where setImmutable is the same logic as set (npSet), but returns an updated copy of the container instead. Under the hood it would use something like immutability-helper to reuse unchanged data when copying.

Or would it perhaps be better to create an alternative library file? For example:

const newContainer = nestedPropertySetImmutable(container, attr, value);

Which would be an immutable conversion of npSet (using immutability-helper). The downside of this approach is that it might create a code fork between it and nestedProperty.

[alexcjohnson]

I like the idea of const newContainer = prop.setImmutable(value);.

It'll take a bit of work to get this integrated with plotly.js, and I'd like to have a clear idea how that will work before we add this to the library. My main concern is that container often isn't really the root of the state tree - instead of data we start at a trace; layout is often used as the container but sometimes a subcontainer like layout.xaxis is used.

[vdh]

@alexcjohnson That shouldn't be a problem, the update can be applied further up the state tree in an immutable fashion as well. For example with immutability-helper:

const prop = nestedProperty(layout.xaxis, attr);
const newXaxis = prop.setImmutable(value);
const newLayout = update(layout, { xaxis: { $set: newXaxis } });

Or with the (ES6) spread operator:

const prop = nestedProperty(layout.xaxis, attr);
const newXaxis = prop.setImmutable(value);
const newLayout = { ...layout, xaxis: newXaxis };

And for an array like data (using an ES6 computed key to save on space):

const trace = data[traceIndex];
const prop = nestedProperty(trace, attr);
const newTrace = prop.setImmutable(value);
const newData = update(data, { [traceIndex]: { $set: newTrace } });

Or getting even fancier with immutability-helper's $apply:

const newData = update(data, {
  [traceIndex]: {
    $apply: (trace) => {
      const prop = nestedProperty(trace, attr);
      return prop.setImmutable(value);
    }
  }
});
const newLayout = update(layout, {
  xaxis: {
    $apply: (xaxis) => {
      const prop = nestedProperty(xaxis, attr);
      return prop.setImmutable(value);
    }
  }
});

Or if you want to get vanilla JS again with an array:

const trace = data[traceIndex];
const prop = nestedProperty(trace, attr);
const newTrace = prop.setImmutable(value);
const newData = data.slice();
newData[traceIndex] = newTrace;

There's other ways to do that last example without mutating, but it's permissible to mutate your own copies as long as you do it in an immutable fashion.

[alexcjohnson]

Sure, I'm just worried that in some of the contexts in which we use nestedProperty the full state tree may not be available, which would require some more significant refactoring. Nothing we can't do, just trying to predict the scope of this project.

[vdh]

Well, passing updates back up to the full state tree would be up to the scope that nestedProperty is used in. Immutable tools never need to worry about state they haven't been given access to.

Adding an immutable option to nestedProperty is more of a stepping stone to making other immutable conversions easier, by enabling the special setter logic of this helper library to be retained without needing to refactor it away entirely.

Another alternative solution might be something to generate an object of immutability-helper commands that relate to the logic that nestedProperty would have done for a given set call, in case someone might want to chain a bunch of commands together into one single update call. But that's just an implementation detail, any type of immutable mode would work fine as the "stepping stone".

[nicolaskruchten]

This is basically the same suggestion I've been making internally for a while, so I'm +1 for using immutability-helper or something like it here :) I'm about to start work on replacing uses of nestedProperty with immutability-helper over in the editor.

The concern I would have is more around situations where right now two disparate bits of plotly.js are essentially communicating using shared references, and this link being broken because one scope is writing to new objects while another scope is reading from a reference to an older, 'orphaned' object. How often does this sort of thing happen, @alexcjohnson ?

[vdh]

@nicolaskruchten Perhaps you could solve this issue by essentially doubling up on changes during the transition. Create a new immutable update object to send back in callbacks, but then continue to mutate the inputs in the original way for legacy code support?

The legacy code (presumably?) would already be ignoring the callback's parameters, but new code can take advantage of them. After that, something like an opt-in immutable config option could then turn off the legacy mutation support once all paths are converted to immutable.

e.g.:

// Before
const prop = nestedProperty(container, attr);
prop.set(value); // container is mutated
callback();

// After
const prop = nestedProperty(container, attr);
const newContainer = prop.setImmutable(value);
if (!immutableMode) {
  prop.set(value); // container is mutated
}
callback(newContainer);

[nicolaskruchten]

That might work in cases where the continuation-passing style has been used but will not address the case I'm thinking of where two bits of synchronous code (i.e. no callback) are being called one after another, say in a loop, and both reading and writing the same object which they hold a reference to that is not the root. Something structurally like:

const layout = {a: {b: {c: 1}}};

const closure1 = function() { 
    const x = layout.a.b; 
    return (q) => nestedProperty(x, 'c').set(q);
}()

const closure2 = function() { 
    const y = layout.a.b; 
    return () => y.c;
}()

for(let i =0; i<5; i++) {
  closure1(i); 
  console.log(closure2());
}

The two closures are communicating via shared but inaccessible references (x===y). Detecting this kind of condition seems very challenging to me, given how deep various call-chains can get, and porting this over to an immutable pattern would require major structural changes...

[gvwilson]

Hi - this issue has been sitting for a while, so as part of our effort to tidy up our public repositories I'm going to close it. If it's still a concern, we'd be grateful if you could open a new issue (with a short reproducible example if appropriate) so that we can add it to our stack. Cheers - @gvwilson