SArray.slice() versus slicing the update value

[asprionj]

I'm currently playing around with S/Surplus because I really like its approach / philosophy. I have a keyed list implemented as Map. I want to show the entries of this map filtered and in a specific order. My idea is to use an SArray containing all keys to render the relevant entries only and in the desired order. In addition, I only want to show a reduced number of entries, but that can be extended by the user. Here's an (almost) minimal example:

import * as Surplus from 'surplus';
import SArray from 's-array';
import data from 'surplus-mixin-data';
import S from 's-js'

var content = new Map([[0,"a"], [1,"b"], [2,"c"], [3,"d"]]);

// initial view: reversely-ordered list, max. 3 entries shown
const showInds = SArray(Array.from(content.keys()).reverse());
var tableLength = S.data(3);

// function to sort the list (alphabetically)
const sortList = () => {
  let newInds = Array.from(content).sort().map(c => c[0]);
  showInds(newInds.slice(0,tableLength())); // <-- limiting number of entries here
}

const view =
  <div>
    <div>
      {showInds.map(key => // <-- mapping over the entire SArray
          <div onClick={ev => deleteOne(key)}>{content.get(key)}</div>)}
    </div>
    <button onclick={(ev) => sortList()}>sort</button>
    <input type="number" min="0" step="1" fn={data(tableLength)}/>
  </div>

document.body.appendChild(view);

That works fine, but a changed number-of-entries setting only takes effect when sorting the list again. So I thought to slice the SArray itself right before mapping over it in the view, i.e. in the sortList function, just use showInds(newInds); and, in the view, use

showInds.slice(0,tableLength()).map(...

This works fine before hitting the sort button. Once sorted, changing the number of entries to be shown (up or down) results in the runtime error

In main.js, that's on the last line of this code snippet:

    function reconcileArrays(parent, ns, us) {
        var ulen = us.length, 
        // n = nodes, u = updates
        // ranges defined by min and max indices
        nmin = 0, nmax = ns.length - 1, umin = 0, umax = ulen - 1, 
        // start nodes of ranges
        n = ns[nmin], u = us[umin], 
        // end nodes of ranges
        nx = ns[nmax], ux = us[umax], 
        // node, if any, just after ux, used for doing .insertBefore() to put nodes at end
        ul = nx.nextSibling, i, j, k, loop = true;

Could very well be that I'm trying to do something in a way it's not meant to be done? However, I was surprised that the two seemingly equivalent solutions don't work the same. Is this a fundamental limitation of the core idea or a bug / missing feature?

[adamhaile]

Sorry for being so slow with a reply. Somehow your report slipped through my radar and I'm just seeing it now :(.

Thanks for including enough details for me to repro the issue. I made a codepen from it, which is here: https://codepen.io/adamhaile/pen/LJMmgq?editors=1000 .

This turns out to be a logic bug in Surplus.content(). The code records what the current state of the node's content is, so that when an update comes in, it can quickly diff against the old state to figure out the minimal set of changes. The way you constructed your code happened to uncover an issue where the code winds up comparing to an outdated version of the state.

The issue arises when:

1. Your {...} expression returns a signal (your code did this with the SArray methods)
2. Your {...} expression also reads a signal that isn't wrapped by the signal you're creating (i.e., if it changes, the return signal is recreated, not just re-evaluated) (your code did this with the reference to tableLength())

Even then, most scenarios come out OK -- we compare against old state, but the difference is harmless. It took a couple more conditions to actually get a bug:

3. The signal returned by the {...} expression carries an array
4. The array signal changes in a way that preserves some but not all of its elements
5. The array signal is then invalidated and recreated with a signal that returns an array which shares at least one of the previous updates' items.

I've got a candidate fix I'm experimenting with locally. I'll post up again once it's past testing and ready to push.

Thanks for the report, and again, sorry for the slow reply!