Problem: Cells were passing around a notebook

[rgbkrk]

Solution: Rely only on cell data per cell

[rgbkrk]

$ npm t

> composition@0.0.1 test /Users/kyle6475/code/src/github.com/nteract/composition
> electron-mocha --renderer --compilers js:babel-core/register 'test/**/*.@(js|jsx)'



  Notebook

    ✓ accepts an Immutable.List of cells (73ms)

  createStore

    ✓ sets up our store model


  2 passing (182ms)

[rgbkrk]

I'd like to follow this one up with removing the need for an index (some sort of ref to a cell). Not sure how to do that, but I think it will lead to less asynchronous errors (what if a cell is in flight and does an update?)

[jdfreder]

I'd like to follow this one up with removing the need for an index (some sort of ref to a cell). Not sure how to do that, but I think it will lead to less asynchronous errors (what if a cell is in flight and does an update?)

So during save/load we'd strip/add these ref guids?

[rgbkrk]

So during save/load we'd strip/add these ref guids?

We'd have some way to go from our internal structure to the disk format. The current way to do it is to take the immutable.js notebook structure and run toJS(). We'd keep the API simple on the outside for our own contributor experience.

As for the internal structure, I'm not sure yet. When a cell gets new data (outputs, source/input, execution_count), the action we're aiming for is saying "this cell has updated" and all we have right now is the index that was supplied to the cell. It's tempting to use a uuid for each cell and map between the list of cells and the current cell version:

notebook = {
  cells: [id1, id2, id3, id4],
  ...
}

cells = {
  id1: cell1,
  id2: cell2,
  ...
}

This seems like it's adding indirection rather than solving the root problem. A cell still needs to know its ID (which could also be the key prop). This at least clarifies that the notebook is maintaining the ordering of cells while the cells map is able to track state of the individual cells over time. We have to maintain two data structures with this.

[lock]

This thread has been automatically locked because it has not had recent activity. Please open a new issue for related bugs and link to relevant comments in this thread.