From 7de909cfe441b38fe42924153ee4b7f1e29fd050 Mon Sep 17 00:00:00 2001 From: Ian Storm Taylor Date: Thu, 16 Nov 2017 11:48:46 -0800 Subject: [PATCH] fix to make lists of operations immutable too --- packages/slate/Changelog.md | 4 +++- packages/slate/src/changes/with-schema.js | 4 ++-- packages/slate/src/models/change.js | 11 ++++++----- packages/slate/src/models/history.js | 9 ++++----- packages/slate/src/models/schema.js | 4 ++-- 5 files changed, 17 insertions(+), 15 deletions(-) diff --git a/packages/slate/Changelog.md b/packages/slate/Changelog.md index 7c72a6a7b..1304e7e4d 100644 --- a/packages/slate/Changelog.md +++ b/packages/slate/Changelog.md @@ -11,7 +11,9 @@ This document maintains a list of changes to the `slate` package with each new v ###### BREAKING -- **The operation objects in Slate are now immutable records.** Previously they were native, mutable Javascript objects. Now, there's a new immutable `Operation` model in Slate, ensuring that all of the data inside `Value` objects are immutable. And it allows for easy serialization of operations using `operation.toJSON()` for when sending them between editors. This should not affect most users, unless you are relying on changing the values of the low-level Slate operations (simply reading them is fine). +- **Operation objects in Slate are now immutable records.** Previously they were native, mutable Javascript objects. Now, there's a new immutable `Operation` model in Slate, ensuring that all of the data inside `Value` objects are immutable. And it allows for easy serialization of operations using `operation.toJSON()` for when sending them between editors. This should not affect most users, unless you are relying on changing the values of the low-level Slate operations (simply reading them is fine). + +- **Operation lists in Slate are now immutable lists.** Previously they were native, mutable Javascript arrays. Now, to keep consistent with other immutable uses, they are immutable lists. This should not affect most users. ###### NEW diff --git a/packages/slate/src/changes/with-schema.js b/packages/slate/src/changes/with-schema.js index 5d44a8772..879fd7209 100644 --- a/packages/slate/src/changes/with-schema.js +++ b/packages/slate/src/changes/with-schema.js @@ -76,7 +76,7 @@ function normalizeNodeAndChildren(change, node, schema) { // While there is still a child key that hasn't been normalized yet... while (keys.length) { - const ops = change.operations.length + const { size } = change.operations let key // PERF: use a mutable set here since we'll be add to it a lot. @@ -95,7 +95,7 @@ function normalizeNodeAndChildren(change, node, schema) { // PERF: Only re-find the node and re-normalize any new children if // operations occured that might have changed it. - if (change.operations.length != ops) { + if (change.operations.size > size) { node = refindNode(change, node) // Add any new children back onto the stack. diff --git a/packages/slate/src/models/change.js b/packages/slate/src/models/change.js index af2c58253..6bae46d01 100644 --- a/packages/slate/src/models/change.js +++ b/packages/slate/src/models/change.js @@ -2,6 +2,7 @@ import Debug from 'debug' import isPlainObject from 'is-plain-object' import pick from 'lodash/pick' +import { List } from 'immutable' import MODEL_TYPES from '../constants/model-types' import Changes from '../changes' @@ -45,7 +46,7 @@ class Change { constructor(attrs) { const { value } = attrs this.value = value - this.operations = [] + this.operations = new List() this.flags = pick(attrs, ['merge', 'save']) } @@ -63,7 +64,7 @@ class Change { * Apply an `operation` to the current value, saving the operation to the * history if needed. * - * @param {Object} operation + * @param {Operation|Object} operation * @param {Object} options * @return {Change} */ @@ -86,7 +87,7 @@ class Change { // Derive the default option values. const { - merge = operations.length == 0 ? null : true, + merge = operations.size == 0 ? null : true, save = true, skip = null, } = options @@ -103,14 +104,14 @@ class Change { // Update the mutable change object. this.value = value - this.operations.push(operation) + this.operations = operations.push(operation) return this } /** * Apply a series of `operations` to the current value. * - * @param {Array} operations + * @param {Array|List} operations * @param {Object} options * @return {Change} */ diff --git a/packages/slate/src/models/history.js b/packages/slate/src/models/history.js index e3205d2a3..d94d220c9 100644 --- a/packages/slate/src/models/history.js +++ b/packages/slate/src/models/history.js @@ -2,7 +2,7 @@ import Debug from 'debug' import isEqual from 'lodash/isEqual' import isPlainObject from 'is-plain-object' -import { Record, Stack } from 'immutable' +import { List, Record, Stack } from 'immutable' import MODEL_TYPES from '../constants/model-types' @@ -113,7 +113,7 @@ class History extends Record(DEFAULTS) { let { undos, redos } = history let { merge, skip } = options const prevBatch = undos.peek() - const prevOperation = prevBatch && prevBatch[prevBatch.length - 1] + const prevOperation = prevBatch && prevBatch.last() if (skip == null) { skip = shouldSkip(operation, prevOperation) @@ -131,15 +131,14 @@ class History extends Record(DEFAULTS) { // If the `merge` flag is true, add the operation to the previous batch. if (merge && prevBatch) { - const batch = prevBatch.slice() - batch.push(operation) + const batch = prevBatch.push(operation) undos = undos.pop() undos = undos.push(batch) } // Otherwise, create a new batch with the operation. else { - const batch = [operation] + const batch = new List([operation]) undos = undos.push(batch) } diff --git a/packages/slate/src/models/schema.js b/packages/slate/src/models/schema.js index 08edea704..8091339fb 100644 --- a/packages/slate/src/models/schema.js +++ b/packages/slate/src/models/schema.js @@ -189,9 +189,9 @@ class Schema extends Record(DEFAULTS) { return (change) => { debug(`normalizing`, { reason, context }) const { rule } = context - const count = change.operations.length + const { size } = change.operations if (rule.normalize) rule.normalize(change, reason, context) - if (change.operations.length > count) return + if (change.operations.size > size) return this.normalize(change, reason, context) } }