Merge pull request #1908 from ian-r-rose/cell_uuid

[WIP] Cell uuid

packages/coreutils/src/undoablevector.ts

@@ -2,7 +2,7 @@
 // Distributed under the terms of the Modified BSD License.
 
 import {
-  JSONObject
+  JSONValue
 } from '@phosphor/coreutils';
 
 import {
@@ -15,14 +15,20 @@ import {
 
 
 /**
- * An object which can be serialized to JSON.
+ * An object which knows how to serialize and
+ * deserialize the type T.
  */
 export
-interface ISerializable {
+interface ISerializer<T> {
   /**
    * Convert the object to JSON.
    */
-  toJSON(): JSONObject;
+  toJSON(value: T): JSONValue;
+
+  /**
+   * Deserialize the object from JSON.
+   */
+  fromJSON(value: JSONValue): T;
 }
 
 
@@ -30,7 +36,7 @@ interface ISerializable {
  * An observable vector that supports undo/redo.
  */
 export
-interface IObservableUndoableVector<T extends ISerializable> extends IObservableVector<T> {
+interface IObservableUndoableVector<T> extends IObservableVector<T> {
   /**
    * Whether the object can redo changes.
    */
@@ -75,13 +81,13 @@ interface IObservableUndoableVector<T extends ISerializable> extends IObservable
  * A concrete implementation of an observable undoable vector.
  */
 export
-class ObservableUndoableVector<T extends ISerializable> extends ObservableVector<T> implements IObservableUndoableVector<T> {
+class ObservableUndoableVector<T> extends ObservableVector<T> implements IObservableUndoableVector<T> {
   /**
    * Construct a new undoable observable vector.
    */
-  constructor(factory: (value: JSONObject) => T) {
+  constructor(serializer: ISerializer<T>) {
     super();
-    this._factory = factory;
+    this._serializer = serializer;
     this.changed.connect(this._onVectorChanged, this);
   }
 
@@ -103,7 +109,7 @@ class ObservableUndoableVector<T extends ISerializable> extends ObservableVector
    * Dispose of the resources held by the model.
    */
   dispose(): void {
-    this._factory = null;
+    this._serializer = null;
     this._stack = null;
     super.dispose();
   }
@@ -201,9 +207,9 @@ class ObservableUndoableVector<T extends ISerializable> extends ObservableVector
   /**
    * Undo a change event.
    */
-  private _undoChange(change: ObservableVector.IChangedArgs<JSONObject>): void {
+  private _undoChange(change: ObservableVector.IChangedArgs<JSONValue>): void {
     let index = 0;
-    let factory = this._factory;
+    let serializer = this._serializer;
     switch (change.type) {
     case 'add':
       each(change.newValues, () => {
@@ -213,13 +219,13 @@ class ObservableUndoableVector<T extends ISerializable> extends ObservableVector
     case 'set':
       index = change.oldIndex;
       each(change.oldValues, value => {
-        this.set(index++, factory(value));
+        this.set(index++, serializer.fromJSON(value));
       });
       break;
     case 'remove':
       index = change.oldIndex;
       each(change.oldValues, value => {
-        this.insert(index++, factory(value));
+        this.insert(index++, serializer.fromJSON(value));
       });
       break;
     case 'move':
@@ -233,20 +239,20 @@ class ObservableUndoableVector<T extends ISerializable> extends ObservableVector
   /**
    * Redo a change event.
    */
-  private _redoChange(change: ObservableVector.IChangedArgs<JSONObject>): void {
+  private _redoChange(change: ObservableVector.IChangedArgs<JSONValue>): void {
     let index = 0;
-    let factory = this._factory;
+    let serializer = this._serializer;
     switch (change.type) {
     case 'add':
       index = change.newIndex;
       each(change.newValues, value => {
-        this.insert(index++, factory(value));
+        this.insert(index++, serializer.fromJSON(value));
       });
       break;
     case 'set':
       index = change.newIndex;
       each(change.newValues, value => {
-        this.set(change.newIndex++, factory(value));
+        this.set(change.newIndex++, serializer.fromJSON(value));
       });
       break;
     case 'remove':
@@ -265,14 +271,14 @@ class ObservableUndoableVector<T extends ISerializable> extends ObservableVector
   /**
    * Copy a change as JSON.
    */
-  private _copyChange(change: ObservableVector.IChangedArgs<T>): ObservableVector.IChangedArgs<JSONObject> {
-    let oldValues: JSONObject[] = [];
+  private _copyChange(change: ObservableVector.IChangedArgs<T>): ObservableVector.IChangedArgs<JSONValue> {
+    let oldValues: JSONValue[] = [];
     each(change.oldValues, value => {
-      oldValues.push(value.toJSON());
+      oldValues.push(this._serializer.toJSON(value));
     });
-    let newValues: JSONObject[] = [];
+    let newValues: JSONValue[] = [];
     each(change.newValues, value => {
-      newValues.push(value.toJSON());
+      newValues.push(this._serializer.toJSON(value));
     });
     return {
       type: change.type,
@@ -287,6 +293,6 @@ class ObservableUndoableVector<T extends ISerializable> extends ObservableVector
   private _isUndoable = true;
   private _madeCompoundChange = false;
   private _index = -1;
-  private _stack: ObservableVector.IChangedArgs<JSONObject>[][] = [];
-  private _factory: (value: JSONObject) => T = null;
+  private _stack: ObservableVector.IChangedArgs<JSONValue>[][] = [];
+  private _serializer: ISerializer<T> = null;
 }

packages/notebook/src/celllist.ts

@@ -0,0 +1,548 @@
+// Copyright (c) Jupyter Development Team.
+// Distributed under the terms of the Modified BSD License.
+
+import {
+  ArrayExt, IIterator, IterableOrArrayLike, each, toArray, ArrayIterator
+} from '@phosphor/algorithm';
+
+import {
+  ISignal, Signal
+} from '@phosphor/signaling';
+
+import {
+  IObservableMap, ObservableMap, IObservableVector, ObservableVector,
+  IObservableUndoableVector, ObservableUndoableVector, uuid
+} from '@jupyterlab/coreutils';
+
+import {
+  ICellModel
+} from '@jupyterlab/cells';
+
+
+/**
+ * A cell list object that supports undo/redo.
+ */
+export
+class CellList implements IObservableUndoableVector<ICellModel> {
+  /**
+   * Construct the cell list.
+   */
+  constructor() {
+    this._cellOrder = new ObservableUndoableVector<string>({
+      toJSON: (val: string) => { return val; },
+      fromJSON: (val: string) => { return val; }
+    });
+    this._cellMap = new ObservableMap<ICellModel>();
+
+    this._cellOrder.changed.connect(this._onOrderChanged, this);
+  }
+
+  /**
+   * A signal emitted when the cell list has changed.
+   */
+  get changed(): ISignal<this, ObservableVector.IChangedArgs<ICellModel>> {
+    return this._changed;
+  }
+
+  /**
+   * Test whether the cell list has been disposed.
+   */
+  get isDisposed(): boolean {
+    return this._isDisposed;
+  }
+
+  /**
+   * Test whether the list is empty.
+   *
+   * @returns `true` if the cell list is empty, `false` otherwise.
+   *
+   * #### Notes
+   * This is a read-only property.
+   *
+   * #### Complexity
+   * Constant.
+   *
+   * #### Iterator Validity
+   * No changes.
+   */
+  get isEmpty(): boolean {
+    return this._cellOrder.length === 0;
+  }
+
+  /**
+   * Get the length of the cell list.
+   *
+   * @return The number of cells in the cell list.
+   *
+   * #### Notes
+   * This is a read-only property.
+   *
+   * #### Complexity
+   * Constant.
+   *
+   * #### Iterator Validity
+   * No changes.
+   */
+  get length(): number {
+    return this._cellOrder.length;
+  }
+
+  /**
+   * Get the cell at the front of the cell list.
+   *
+   * @returns The cell at the front of the cell list, or `undefined` if
+   *   the cell list is empty.
+   *
+   * #### Notes
+   * This is a read-only property.
+   *
+   * #### Complexity
+   * Constant.
+   *
+   * #### Iterator Validity
+   * No changes.
+   */
+  get front(): ICellModel {
+    return this.at(0);
+  }
+
+  /**
+   * Get the cell at the back of the cell list.
+   *
+   * @returns The cell at the back of the cell list, or `undefined` if
+   *   the cell list is empty.
+   *
+   * #### Notes
+   * This is a read-only property.
+   *
+   * #### Complexity
+   * Constant.
+   *
+   * #### Iterator Validity
+   * No changes.
+   */
+  get back(): ICellModel {
+    return this.at(this.length-1);
+  }
+
+  /**
+   * Create an iterator over the cells in the cell list.
+   *
+   * @returns A new iterator starting at the front of the cell list.
+   *
+   * #### Complexity
+   * Constant.
+   *
+   * #### Iterator Validity
+   * No changes.
+   */
+  iter(): IIterator<ICellModel> {
+    let arr: ICellModel[] = [];
+    for (let id of toArray(this._cellOrder)) {
+      arr.push(this._cellMap.get(id));
+    }
+    return new ArrayIterator<ICellModel>(arr);
+  }
+
+  /**
+   * Dispose of the resources held by the cell list.
+   */
+  dispose(): void {
+    if (this._isDisposed) {
+      return;
+    }
+    this._isDisposed = true;
+    Signal.clearData(this);
+    // Clean up the cell map and cell order objects.
+    for (let cell of this._cellMap.values()) {
+      cell.dispose();
+    }
+    this._cellMap.dispose();
+    this._cellOrder.dispose();
+  }
+
+  /**
+   * Get the cell at the specified index.
+   *
+   * @param index - The positive integer index of interest.
+   *
+   * @returns The cell at the specified index.
+   *
+   * #### Complexity
+   * Constant.
+   *
+   * #### Iterator Validity
+   * No changes.
+   *
+   * #### Undefined Behavior
+   * An `index` which is non-integral or out of range.
+   */
+  at(index: number): ICellModel {
+    return this._cellMap.get(this._cellOrder.at(index)) as ICellModel;
+  }
+
+  /**
+   * Set the cell at the specified index.
+   *
+   * @param index - The positive integer index of interest.
+   *
+   * @param cell - The cell to set at the specified index.
+   *
+   * #### Complexity
+   * Constant.
+   *
+   * #### Iterator Validity
+   * No changes.
+   *
+   * #### Undefined Behavior
+   * An `index` which is non-integral or out of range.
+   *
+   * #### Notes
+   * This should be considered to transfer ownership of the
+   * cell to the `CellList`. As such, `cell.dispose()` should
+   * not be called by other actors.
+   */
+  set(index: number, cell: ICellModel): void {
+    // Generate a new uuid for the cell.
+    let id = uuid();
+    // Set the internal data structures.
+    this._cellMap.set(id, cell);
+    this._cellOrder.set(index, id);
+  }
+
+  /**
+   * Add a cell to the back of the cell list.
+   *
+   * @param cell - The cell to add to the back of the cell list.
+   *
+   * @returns The new length of the cell list.
+   *
+   * #### Complexity
+   * Constant.
+   *
+   * #### Iterator Validity
+   * No changes.
+   *
+   * #### Notes
+   * This should be considered to transfer ownership of the
+   * cell to the `CellList`. As such, `cell.dispose()` should
+   * not be called by other actors.
+   */
+  pushBack(cell: ICellModel): number {
+    // Generate a new uuid for the cell.
+    let id = uuid();
+    // Set the internal data structures.
+    this._cellMap.set(id, cell);
+    let num = this._cellOrder.pushBack(id);
+    return num;
+  }
+
+  /**
+   * Remove and return the cell at the back of the cell list.
+   *
+   * @returns The cell at the back of the cell list, or `undefined` if
+   *   the cell list is empty.
+   *
+   * #### Complexity
+   * Constant.
+   *
+   * #### Iterator Validity
+   * Iterators pointing at the removed cell are invalidated.
+   */
+  popBack(): ICellModel {
+    //Don't clear the map in case we need to reinstate the cell
+    let id = this._cellOrder.popBack();
+    let cell = this._cellMap.get(id);
+    return cell;
+  }
+
+  /**
+   * Insert a cell into the cell list at a specific index.
+   *
+   * @param index - The index at which to insert the cell.
+   *
+   * @param cell - The cell to set at the specified index.
+   *
+   * @returns The new length of the cell list.
+   *
+   * #### Complexity
+   * Linear.
+   *
+   * #### Iterator Validity
+   * No changes.
+   *
+   * #### Notes
+   * The `index` will be clamped to the bounds of the cell list.
+   *
+   * #### Undefined Behavior
+   * An `index` which is non-integral.
+   *
+   * #### Notes
+   * This should be considered to transfer ownership of the
+   * cell to the `CellList`. As such, `cell.dispose()` should
+   * not be called by other actors.
+   */
+  insert(index: number, cell: ICellModel): number {
+    // Generate a new uuid for the cell.
+    let id = uuid();
+    // Set the internal data structures.
+    this._cellMap.set(id, cell);
+    let num = this._cellOrder.insert(index, id);
+    return num;
+  }
+
+  /**
+   * Remove the first occurrence of a cell from the cell list.
+   *
+   * @param cell - The cell of interest.
+   *
+   * @returns The index of the removed cell, or `-1` if the cell
+   *   is not contained in the cell list.
+   *
+   * #### Complexity
+   * Linear.
+   *
+   * #### Iterator Validity
+   * Iterators pointing at the removed cell and beyond are invalidated.
+   */
+  remove(cell: ICellModel): number {
+    let index = ArrayExt.findFirstIndex(
+      toArray(this._cellOrder), id => (this._cellMap.get(id)===cell));
+    this.removeAt(index);
+    return index;
+  }
+
+  /**
+   * Remove and return the cell at a specific index.
+   *
+   * @param index - The index of the cell of interest.
+   *
+   * @returns The cell at the specified index, or `undefined` if the
+   *   index is out of range.
+   *
+   * #### Complexity
+   * Constant.
+   *
+   * #### Iterator Validity
+   * Iterators pointing at the removed cell and beyond are invalidated.
+   *
+   * #### Undefined Behavior
+   * An `index` which is non-integral.
+   */
+  removeAt(index: number): ICellModel {
+    let id= this._cellOrder.removeAt(index);
+    let cell = this._cellMap.get(id);
+    return cell;
+  }
+
+  /**
+   * Remove all cells from the cell list.
+   *
+   * #### Complexity
+   * Linear.
+   *
+   * #### Iterator Validity
+   * All current iterators are invalidated.
+   */
+  clear(): void {
+    this._cellOrder.clear();
+  }
+
+  /**
+   * Move a cell from one index to another.
+   *
+   * @parm fromIndex - The index of the element to move.
+   *
+   * @param toIndex - The index to move the element to.
+   *
+   * #### Complexity
+   * Constant.
+   *
+   * #### Iterator Validity
+   * Iterators pointing at the lesser of the `fromIndex` and the `toIndex`
+   * and beyond are invalidated.
+   *
+   * #### Undefined Behavior
+   * A `fromIndex` or a `toIndex` which is non-integral.
+   */
+  move(fromIndex: number, toIndex: number): void {
+    this._cellOrder.move(fromIndex, toIndex);
+  }
+
+  /**
+   * Push a set of cells to the back of the cell list.
+   *
+   * @param cells - An iterable or array-like set of cells to add.
+   *
+   * @returns The new length of the cell list.
+   *
+   * #### Complexity
+   * Linear.
+   *
+   * #### Iterator Validity
+   * No changes.
+   *
+   * #### Notes
+   * This should be considered to transfer ownership of the
+   * cells to the `CellList`. As such, `cell.dispose()` should
+   * not be called by other actors.
+   */
+  pushAll(cells: IterableOrArrayLike<ICellModel>): number {
+    let newValues = toArray(cells);
+    each(newValues, cell => {
+      // Generate a new uuid for the cell.
+      let id = uuid();
+      // Set the internal data structures.
+      this._cellMap.set(id, cell);
+      this._cellOrder.pushBack(id);
+    });
+    return this.length;
+  }
+
+  /**
+   * Insert a set of items into the cell list at the specified index.
+   *
+   * @param index - The index at which to insert the cells.
+   *
+   * @param cells - The cells to insert at the specified index.
+   *
+   * @returns The new length of the cell list.
+   *
+   * #### Complexity.
+   * Linear.
+   *
+   * #### Iterator Validity
+   * No changes.
+   *
+   * #### Notes
+   * The `index` will be clamped to the bounds of the cell list.
+   *
+   * #### Undefined Behavior.
+   * An `index` which is non-integral.
+   *
+   * #### Notes
+   * This should be considered to transfer ownership of the
+   * cells to the `CellList`. As such, `cell.dispose()` should
+   * not be called by other actors.
+   */
+  insertAll(index: number, cells: IterableOrArrayLike<ICellModel>): number {
+    let newValues = toArray(cells);
+    each(newValues, cell => {
+      // Generate a new uuid for the cell.
+      let id = uuid();
+      this._cellMap.set(id, cell);
+      this._cellOrder.beginCompoundOperation();
+      this._cellOrder.insert(index++, id);
+      this._cellOrder.endCompoundOperation();
+    });
+    return this.length;
+  }
+
+  /**
+   * Remove a range of items from the cell list.
+   *
+   * @param startIndex - The start index of the range to remove (inclusive).
+   *
+   * @param endIndex - The end index of the range to remove (exclusive).
+   *
+   * @returns The new length of the cell list.
+   *
+   * #### Complexity
+   * Linear.
+   *
+   * #### Iterator Validity
+   * Iterators pointing to the first removed cell and beyond are invalid.
+   *
+   * #### Undefined Behavior
+   * A `startIndex` or `endIndex` which is non-integral.
+   */
+  removeRange(startIndex: number, endIndex: number): number {
+    this._cellOrder.removeRange(startIndex, endIndex);
+    return this.length;
+  }
+
+  /**
+   * Whether the object can redo changes.
+   */
+  get canRedo(): boolean {
+    return this._cellOrder.canRedo;
+  }
+
+  /**
+   * Whether the object can undo changes.
+   */
+  get canUndo(): boolean {
+    return this._cellOrder.canUndo;
+  }
+
+  /**
+   * Begin a compound operation.
+   *
+   * @param isUndoAble - Whether the operation is undoable.
+   *   The default is `true`.
+   */
+  beginCompoundOperation(isUndoAble?: boolean): void {
+    this._cellOrder.beginCompoundOperation(isUndoAble);
+  }
+
+  /**
+   * End a compound operation.
+   */
+  endCompoundOperation(): void {
+    this._cellOrder.endCompoundOperation();
+  }
+
+  /**
+   * Undo an operation.
+   */
+  undo(): void {
+    this._cellOrder.undo();
+  }
+
+  /**
+   * Redo an operation.
+   */
+  redo(): void {
+    this._cellOrder.redo();
+  }
+
+  /**
+   * Clear the change stack.
+   */
+  clearUndo(): void {
+    // Dispose of cells not in the current
+    // cell order.
+    for (let key of this._cellMap.keys()) {
+      if (ArrayExt.findFirstIndex(
+         toArray(this._cellOrder), id => id===key) === -1) {
+        let cell = this._cellMap.get(key) as ICellModel;
+        cell.dispose();
+        this._cellMap.delete(key);
+      }
+    }
+    this._cellOrder.clearUndo();
+  }
+
+  private _onOrderChanged(order: IObservableVector<string>, change: ObservableVector.IChangedArgs<string>): void {
+    let newValues: ICellModel[] = [];
+    let oldValues: ICellModel[] = [];
+    each(change.newValues, (id)=>{
+      newValues.push(this._cellMap.get(id));
+    });
+    each(change.oldValues, (id)=>{
+      oldValues.push(this._cellMap.get(id));
+    });
+    this._changed.emit({
+      type: change.type,
+      oldIndex: change.oldIndex,
+      newIndex: change.newIndex,
+      oldValues,
+      newValues
+    });
+  }
+
+  private _isDisposed: boolean = false;
+  private _cellOrder: IObservableUndoableVector<string> = null;
+  private _cellMap: IObservableMap<ICellModel> = null;
+  private _changed = new Signal<this, ObservableVector.IChangedArgs<ICellModel>>(this);
+}

packages/notebook/src/model.ts

@@ -16,9 +16,13 @@ import {
 
 import {
   IObservableJSON, ObservableJSON, IObservableUndoableVector,
-  ObservableUndoableVector, IObservableVector, ObservableVector, nbformat
+  IObservableVector, ObservableVector, nbformat
 } from '@jupyterlab/coreutils';
 
+import {
+  CellList
+} from './celllist';
+
 
 /**
  * The definition of a model object for a notebook widget.
@@ -66,16 +70,7 @@ class NotebookModel extends DocumentModel implements INotebookModel {
       options.contentFactory || NotebookModel.defaultContentFactory
     );
     this.contentFactory = factory;
-    this._cells = new ObservableUndoableVector<ICellModel>((cell: nbformat.IBaseCell) => {
-      switch (cell.cell_type) {
-        case 'code':
-          return factory.createCodeCell({ cell });
-        case 'markdown':
-          return factory.createMarkdownCell({ cell });
-        default:
-          return factory.createRawCell({ cell });
-      }
-    });
+    this._cells = new CellList();
     // Add an initial code cell by default.
     this._cells.pushBack(factory.createCodeCell({}));
     this._cells.changed.connect(this._onCellsChanged, this);
@@ -147,10 +142,6 @@ class NotebookModel extends DocumentModel implements INotebookModel {
     let cells = this._cells;
     this._cells = null;
     this._metadata.dispose();
-    for (let i = 0; i < cells.length; i++) {
-      let cell = cells.at(i);
-      cell.dispose();
-    }
     cells.dispose();
     super.dispose();
   }
@@ -264,7 +255,6 @@ class NotebookModel extends DocumentModel implements INotebookModel {
       break;
     case 'remove':
       each(change.oldValues, cell => {
-        cell.dispose();
       });
       break;
     case 'set':
@@ -272,7 +262,6 @@ class NotebookModel extends DocumentModel implements INotebookModel {
         cell.contentChanged.connect(this.triggerContentChange, this);
       });
       each(change.oldValues, cell => {
-        cell.dispose();
       });
       break;
     default:

test/src/coreutils/undoablevector.spec.ts

@@ -8,18 +8,17 @@ import {
 } from '@phosphor/coreutils';
 
 import {
-  ObservableUndoableVector, ISerializable
+  ObservableUndoableVector, ISerializer
 } from '@jupyterlab/coreutils';
 
 
 
-class Test implements ISerializable {
-
+class Test {
   constructor(value: JSONObject) {
     this._value = value;
   }
 
-  toJSON(): JSONObject {
+  get value(): JSONObject {
     return this._value;
   }
 
@@ -29,12 +28,18 @@ class Test implements ISerializable {
 
 let count = 0;
 
-function factory(value: JSONObject): Test {
-  value['count'] = count++;
-  return new Test(value);
-}
+class Serializer implements ISerializer<Test> {
+  fromJSON(value: JSONObject): Test {
+    value['count'] = count++;
+    return new Test(value);
+  }
 
+  toJSON(value: Test): JSONObject {
+    return value.value;
+  }
+}
 
+const serializer = new Serializer();
 const value: JSONObject = { name: 'foo' };
 
 
@@ -45,7 +50,7 @@ describe('notebook/common/undo', () => {
     describe('#constructor', () => {
 
       it('should create a new ObservableUndoableVector', () => {
-        let vector = new ObservableUndoableVector(factory);
+        let vector = new ObservableUndoableVector(serializer);
         expect(vector).to.be.an(ObservableUndoableVector);
       });
 
@@ -54,12 +59,12 @@ describe('notebook/common/undo', () => {
     describe('#canRedo', () => {
 
       it('should return false if there is no history', () => {
-        let vector = new ObservableUndoableVector(factory);
+        let vector = new ObservableUndoableVector(serializer);
         expect(vector.canRedo).to.be(false);
       });
 
       it('should return true if there is an undo that can be redone', () => {
-        let vector = new ObservableUndoableVector(factory);
+        let vector = new ObservableUndoableVector(serializer);
         vector.pushBack(new Test(value));
         vector.undo();
         expect(vector.canRedo).to.be(true);
@@ -70,13 +75,13 @@ describe('notebook/common/undo', () => {
     describe('#canUndo', () => {
 
       it('should return false if there is no history', () => {
-        let vector = new ObservableUndoableVector(factory);
+        let vector = new ObservableUndoableVector(serializer);
         expect(vector.canUndo).to.be(false);
       });
 
       it('should return true if there is a change that can be undone', () => {
-        let vector = new ObservableUndoableVector(factory);
-        vector.pushBack(factory(value));
+        let vector = new ObservableUndoableVector(serializer);
+        vector.pushBack(serializer.fromJSON(value));
         expect(vector.canUndo).to.be(true);
       });
 
@@ -85,7 +90,7 @@ describe('notebook/common/undo', () => {
     describe('#dispose()', () => {
 
       it('should dispose of the resources used by the vector', () => {
-        let vector = new ObservableUndoableVector(factory);
+        let vector = new ObservableUndoableVector(serializer);
         vector.dispose();
         expect(vector.isDisposed).to.be(true);
         vector.dispose();
@@ -97,10 +102,10 @@ describe('notebook/common/undo', () => {
     describe('#beginCompoundOperation()', () => {
 
       it('should begin a compound operation', () => {
-        let vector = new ObservableUndoableVector(factory);
+        let vector = new ObservableUndoableVector(serializer);
         vector.beginCompoundOperation();
-        vector.pushBack(factory(value));
-        vector.pushBack(factory(value));
+        vector.pushBack(serializer.fromJSON(value));
+        vector.pushBack(serializer.fromJSON(value));
         vector.endCompoundOperation();
         expect(vector.canUndo).to.be(true);
         vector.undo();
@@ -108,10 +113,10 @@ describe('notebook/common/undo', () => {
       });
 
       it('should not be undoable if isUndoAble is set to false', () => {
-        let vector = new ObservableUndoableVector(factory);
+        let vector = new ObservableUndoableVector(serializer);
         vector.beginCompoundOperation(false);
-        vector.pushBack(factory(value));
-        vector.pushBack(factory(value));
+        vector.pushBack(serializer.fromJSON(value));
+        vector.pushBack(serializer.fromJSON(value));
         vector.endCompoundOperation();
         expect(vector.canUndo).to.be(false);
       });
@@ -121,10 +126,10 @@ describe('notebook/common/undo', () => {
     describe('#endCompoundOperation()', () => {
 
       it('should end a compound operation', () => {
-        let vector = new ObservableUndoableVector(factory);
+        let vector = new ObservableUndoableVector(serializer);
         vector.beginCompoundOperation();
-        vector.pushBack(factory(value));
-        vector.pushBack(factory(value));
+        vector.pushBack(serializer.fromJSON(value));
+        vector.pushBack(serializer.fromJSON(value));
         vector.endCompoundOperation();
         expect(vector.canUndo).to.be(true);
         vector.undo();
@@ -136,39 +141,47 @@ describe('notebook/common/undo', () => {
     describe('#undo()', () => {
 
       it('should undo a pushBack', () => {
-        let vector = new ObservableUndoableVector(factory);
-        vector.pushBack(factory(value));
+        let vector = new ObservableUndoableVector(serializer);
+        vector.pushBack(serializer.fromJSON(value));
         vector.undo();
         expect(vector.length).to.be(0);
       });
 
       it('should undo a pushAll', () => {
-        let vector = new ObservableUndoableVector(factory);
-        vector.pushAll([factory(value), factory(value)]);
+        let vector = new ObservableUndoableVector(serializer);
+        vector.pushAll([serializer.fromJSON(value),
+                        serializer.fromJSON(value)]);
         vector.undo();
         expect(vector.length).to.be(0);
       });
 
       it('should undo a remove', () => {
-         let vector = new ObservableUndoableVector(factory);
-         vector.pushAll([factory(value), factory(value)]);
+         let vector = new ObservableUndoableVector(serializer);
+         vector.pushAll([serializer.fromJSON(value),
+                         serializer.fromJSON(value)]);
          vector.removeAt(0);
          vector.undo();
          expect(vector.length).to.be(2);
       });
 
       it('should undo a removeRange', () => {
-        let vector = new ObservableUndoableVector(factory);
-        vector.pushAll([factory(value), factory(value), factory(value),
-          factory(value), factory(value), factory(value)]);
+        let vector = new ObservableUndoableVector(serializer);
+        vector.pushAll([serializer.fromJSON(value),
+                        serializer.fromJSON(value),
+                        serializer.fromJSON(value),
+                        serializer.fromJSON(value),
+                        serializer.fromJSON(value),
+                        serializer.fromJSON(value)]);
         vector.removeRange(1, 3);
         vector.undo();
         expect(vector.length).to.be(6);
       });
 
       it('should undo a move', () => {
-        let items = [factory(value), factory(value), factory(value)];
-        let vector = new ObservableUndoableVector(factory);
+        let items = [serializer.fromJSON(value),
+                     serializer.fromJSON(value),
+                     serializer.fromJSON(value)];
+        let vector = new ObservableUndoableVector(serializer);
         vector.pushAll(items);
         vector.move(1, 2);
         vector.undo();
@@ -180,24 +193,26 @@ describe('notebook/common/undo', () => {
     describe('#redo()', () => {
 
       it('should redo a pushBack', () => {
-        let vector = new ObservableUndoableVector(factory);
-        vector.pushBack(factory(value));
+        let vector = new ObservableUndoableVector(serializer);
+        vector.pushBack(serializer.fromJSON(value));
         vector.undo();
         vector.redo();
         expect(vector.length).to.be(1);
       });
 
       it('should redo a pushAll', () => {
-        let vector = new ObservableUndoableVector(factory);
-        vector.pushAll([factory(value), factory(value)]);
+        let vector = new ObservableUndoableVector(serializer);
+        vector.pushAll([serializer.fromJSON(value),
+                        serializer.fromJSON(value)]);
         vector.undo();
         vector.redo();
         expect(vector.length).to.be(2);
       });
 
       it('should redo a remove', () => {
-         let vector = new ObservableUndoableVector(factory);
-         vector.pushAll([factory(value), factory(value)]);
+         let vector = new ObservableUndoableVector(serializer);
+         vector.pushAll([serializer.fromJSON(value),
+                         serializer.fromJSON(value)]);
          vector.removeAt(0);
          vector.undo();
          vector.redo();
@@ -205,9 +220,13 @@ describe('notebook/common/undo', () => {
       });
 
       it('should redo a removeRange', () => {
-        let vector = new ObservableUndoableVector(factory);
-        vector.pushAll([factory(value), factory(value), factory(value),
-          factory(value), factory(value), factory(value)]);
+        let vector = new ObservableUndoableVector(serializer);
+        vector.pushAll([serializer.fromJSON(value),
+                        serializer.fromJSON(value),
+                        serializer.fromJSON(value),
+                        serializer.fromJSON(value),
+                        serializer.fromJSON(value),
+                        serializer.fromJSON(value)]);
         vector.removeRange(1, 3);
         vector.undo();
         vector.redo();
@@ -215,8 +234,10 @@ describe('notebook/common/undo', () => {
       });
 
       it('should undo a move', () => {
-        let items = [factory(value), factory(value), factory(value)];
-        let vector = new ObservableUndoableVector(factory);
+        let items = [serializer.fromJSON(value),
+                     serializer.fromJSON(value),
+                     serializer.fromJSON(value)];
+        let vector = new ObservableUndoableVector(serializer);
         vector.pushAll(items);
         vector.move(1, 2);
         vector.undo();
@@ -229,8 +250,8 @@ describe('notebook/common/undo', () => {
     describe('#clearUndo()', () => {
 
       it('should clear the undo stack', () => {
-        let vector = new ObservableUndoableVector(factory);
-        vector.pushBack(factory(value));
+        let vector = new ObservableUndoableVector(serializer);
+        vector.pushBack(serializer.fromJSON(value));
         vector.clearUndo();
         expect(vector.canUndo).to.be(false);
       });

test/src/notebook/model.spec.ts

@@ -108,7 +108,7 @@ describe('notebook/notebook/model', () => {
         model.cells.undo();
         expect(model.cells.length).to.be(2);
         expect(model.cells.at(1).value.text).to.be('foo');
-        expect(model.cells.at(1)).to.not.be(cell);  // should be a clone.
+        expect(model.cells.at(1)).to.be(cell);  // should be ===.
       });
 
       context('cells `changed` signal', () => {
@@ -129,14 +129,6 @@ describe('notebook/notebook/model', () => {
           expect(model.dirty).to.be(true);
         });
 
-        it('should dispose of old cells', () => {
-          let model = new NotebookModel();
-          let cell = model.contentFactory.createCodeCell({});
-          model.cells.pushBack(cell);
-          model.cells.clear();
-          expect(cell.isDisposed).to.be(true);
-        });
-
         it('should add a new code cell when cells are cleared', (done) => {
           let model = new NotebookModel();
           model.cells.clear();

test/src/notebook/widget.spec.ts

@@ -282,7 +282,7 @@ describe('notebook/widget', () => {
           let cell = widget.model.cells.at(1);
           let child = widget.widgets[1];
           widget.model.cells.remove(cell);
-          expect(cell.isDisposed).to.be(true);
+          expect(cell.isDisposed).to.be(false);
           expect(child.isDisposed).to.be(true);
         });