From 13f01be64f92b3c9a693b40506c4c2972ccfe4eb Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 19 Sep 2016 15:26:11 -0700 Subject: [PATCH] Enable text updates in ReactNoop We'll enable updating of text nodes. To be able to test that we need the text nodes to be mutable. They're currently just strings in the Noop renderer so this makes them an object instead. That exposed a bug in ReactFiberCommitWork for text nodes. --- src/renderers/noop/ReactNoop.js | 16 ++++++++++------ .../shared/fiber/ReactFiberCommitWork.js | 8 +++----- .../ReactIncrementalSideEffects-test.js | 1 + 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/renderers/noop/ReactNoop.js b/src/renderers/noop/ReactNoop.js index d8f48f9219..a453b5f57c 100644 --- a/src/renderers/noop/ReactNoop.js +++ b/src/renderers/noop/ReactNoop.js @@ -32,11 +32,12 @@ var scheduledAnimationCallback = null; var scheduledDeferredCallback = null; const TERMINAL_TAG = 99; +const TEXT_TAG = 98; type Container = { rootID: number, children: Array }; type Props = { prop: any }; type Instance = { tag: 99, type: string, id: number, children: Array, prop: any }; -type TextInstance = string; +type TextInstance = { tag: 98, text: string }; var instanceCounter = 0; @@ -44,7 +45,7 @@ function recursivelyAppendChildren(flatArray : Array, c if (!child) { return; } - if (typeof child === 'string' || child.tag === TERMINAL_TAG) { + if (child.tag === TERMINAL_TAG || child.tag === TEXT_TAG) { flatArray.push(child); } else { let node = child; @@ -93,11 +94,14 @@ var NoopRenderer = ReactFiberReconciler({ }, createTextInstance(text : string) : TextInstance { - return text; + var inst = { tag: TEXT_TAG, text : text }; + // Hide from unit tests + Object.defineProperty(inst, 'tag', { value: inst.tag, enumerable: false }); + return inst; }, commitTextUpdate(textInstance : TextInstance, oldText : string, newText : string) : void { - // Not yet supported. + textInstance.text = newText; }, scheduleAnimationCallback(callback) { @@ -178,8 +182,8 @@ var ReactNoop = { function logHostInstances(children: Array, depth) { for (var i = 0; i < children.length; i++) { var child = children[i]; - if (typeof child === 'string') { - log(' '.repeat(depth) + '- ' + child); + if (child.tag === TEXT_TAG) { + log(' '.repeat(depth) + '- ' + child.text); } else { log(' '.repeat(depth) + '- ' + child.type + '#' + child.id); logHostInstances(child.children, depth + 1); diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js index fe2b7cba93..1465086282 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -74,13 +74,11 @@ module.exports = function(config : HostConfig) { if (finishedWork.stateNode == null || !current) { throw new Error('This should only be done during updates.'); } - // TODO: This never gets called yet because I don't have update support - // for text nodes. This only gets updated through a host component or - // container updating with this as one of its child nodes. const textInstance : TI = finishedWork.stateNode; - const oldText : string = finishedWork.memoizedProps; - const newText : string = current.memoizedProps; + const newText : string = finishedWork.memoizedProps; + const oldText : string = current.memoizedProps; commitTextUpdate(textInstance, oldText, newText); + return; } default: throw new Error('This unit of work tag should not have side-effects.'); diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js index 651f93e8cc..1c3a14a2dc 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js @@ -21,6 +21,7 @@ describe('ReactIncrementalSideEffects', () => { }); function div(...children) { + children = children.map(c => typeof c === 'string' ? { text: c } : c); return { type: 'div', children, prop: undefined }; }