diff --git a/src/valdi_modules/src/valdi/valdi_core/src/Remember.ts b/src/valdi_modules/src/valdi/valdi_core/src/Remember.ts new file mode 100644 index 00000000..0891b6c0 --- /dev/null +++ b/src/valdi_modules/src/valdi/valdi_core/src/Remember.ts @@ -0,0 +1,5 @@ +import { getRenderer } from './Renderer'; + +export function remember(factory: () => T, ...keys: unknown[]): T { + return getRenderer().remember(factory, keys); +} diff --git a/src/valdi_modules/src/valdi/valdi_core/src/Renderer.ts b/src/valdi_modules/src/valdi/valdi_core/src/Renderer.ts index 092d9abd..80559371 100644 --- a/src/valdi_modules/src/valdi/valdi_core/src/Renderer.ts +++ b/src/valdi_modules/src/valdi/valdi_core/src/Renderer.ts @@ -33,6 +33,16 @@ import { trace } from './utils/Trace'; const EMPTY_OBJECT = Object.freeze({}); const EMPTY_ARRAY = Object.freeze([]) as []; +interface RememberSlot { + value: T; + keys: unknown[]; +} + +interface RememberState { + slots: RememberSlot[]; + slotIndex: number; +} + interface NodeChildren { childByKey: { [key: string]: T }; children: T[]; @@ -67,6 +77,8 @@ interface VirtualNode extends Node { // Bridge instance, will be set if the virtual node was requested externally. bridge?: VirtualNodeBridge; + + rememberState?: RememberState; } interface RenderedElement { @@ -414,6 +426,21 @@ function getNodeDescription(node: VirtualNode): string { return 'unknown node'; } +function rememberKeysEqual(left: unknown[], right: unknown[]): boolean { + const length = left.length; + if (length !== right.length) { + return false; + } + + for (let i = 0; i < length; i++) { + if (!Object.is(left[i], right[i])) { + return false; + } + } + + return true; +} + interface RendererLogInfo { treeId: string; componentRerendersCount: number; @@ -921,7 +948,7 @@ export class Renderer implements IRenderer { const currentNode = this.getCurrentNode(); const resolvedKey = key || nodePrototype.id; - const resolvedNode = this.resolveVirtualNode(currentNode, resolvedKey, undefined, undefined); + const resolvedNode = this.resolveVirtualNode(currentNode, resolvedKey, undefined, undefined, undefined); let justCreated = false; @@ -1050,6 +1077,9 @@ export class Renderer implements IRenderer { if (node.children) { node.children.insertionIndex = 0; } + if (node.rememberState) { + node.rememberState.slotIndex = 0; + } node.lastRenderId = this.renderId; this.currentNode = node; @@ -1375,10 +1405,12 @@ export class Renderer implements IRenderer { private resolveVirtualNode( parent: VirtualNode, - resolvedKey: string, + key: string, + duplicateKeyIndex: number | undefined, componentConstructor: ComponentConstructor | undefined, componentPrototype: ComponentPrototype | undefined, ): VirtualNode { + const resolvedKey = duplicateKeyIndex !== undefined ? `${key}-${duplicateKeyIndex}` : key; let resolvedNode: VirtualNode | undefined; let children = parent.children; if (!children) { @@ -1429,7 +1461,8 @@ export class Renderer implements IRenderer { const duplicateKeyIndex = children.insertionIndex - resolvedNode.parentIndex + 1; return this.resolveVirtualNode( parent, - resolvedKey + duplicateKeyIndex, + key, + duplicateKeyIndex, componentConstructor, componentPrototype, ); @@ -1457,6 +1490,9 @@ export class Renderer implements IRenderer { private destroyVirtualNode(node: VirtualNode, parentElementWasDestroyed: boolean) { node.parent = undefined; + if (node.rememberState) { + node.rememberState = undefined; + } if (node.element) { const element = node.element; @@ -1611,6 +1647,36 @@ export class Renderer implements IRenderer { } } + remember(factory: () => T, keys: unknown[]): T { + const currentNode = this.currentNode; + if (!currentNode) { + throw Error('Cannot call this outside of a onRender callback'); + } + + let rememberState = currentNode.rememberState; + if (!rememberState) { + rememberState = { + slots: [], + slotIndex: 0, + }; + currentNode.rememberState = rememberState; + } + + const slotIndex = rememberState.slotIndex++; + const slots = rememberState.slots; + const slot = slots[slotIndex] as RememberSlot | undefined; + if (slot && rememberKeysEqual(slot.keys, keys)) { + return slot.value; + } + + const value = factory(); + slots[slotIndex] = { + value, + keys: keys.length ? keys : EMPTY_ARRAY, + }; + return value; + } + hasInjectedAttribute(name: string): boolean { const component = this.getCurrentComponent(); const attributes = component.injectedAttributes; @@ -1835,7 +1901,7 @@ export class Renderer implements IRenderer { const resolvedKey = key || prototype.id; const parent = this.getCurrentNode(); - const resolvedNode = this.resolveVirtualNode(parent, resolvedKey, ctr, prototype); + const resolvedNode = this.resolveVirtualNode(parent, resolvedKey, undefined, ctr, prototype); let justCreated = false; @@ -2161,7 +2227,7 @@ export class Renderer implements IRenderer { beginSlot(slotData: ComponentSlotData) { const parentNode = this.getCurrentNode(); - const node = this.resolveVirtualNode(parentNode, slotData.nodeKey, undefined, undefined); + const node = this.resolveVirtualNode(parentNode, slotData.nodeKey, undefined, undefined, undefined); slotData.node = node; node.slot = true; this.pushVirtualNode(node); diff --git a/src/valdi_modules/src/valdi/valdi_test/test/Remember.spec.ts b/src/valdi_modules/src/valdi/valdi_test/test/Remember.spec.ts new file mode 100644 index 00000000..628d790c --- /dev/null +++ b/src/valdi_modules/src/valdi/valdi_test/test/Remember.spec.ts @@ -0,0 +1,212 @@ +import { NodePrototype } from 'valdi_core/src/NodePrototype'; +import { remember } from 'valdi_core/src/Remember'; +import { Renderer } from 'valdi_core/src/Renderer'; +import 'jasmine/src/jasmine'; +import { RendererTestDelegate } from './RendererTestDelegate'; + +interface TestVirtualNode { + rememberState?: unknown; + children?: { + children: TestVirtualNode[]; + }; +} + +interface RememberedValue { + id: number; + itemKey?: string; +} + +function makeRenderer(): Renderer { + return new Renderer('', undefined, new RendererTestDelegate()); +} + +function makeNodePrototype(viewClass: string): NodePrototype { + return new NodePrototype(viewClass, viewClass); +} + +function hasRememberStorage(node: TestVirtualNode): boolean { + if (Object.prototype.hasOwnProperty.call(node, 'rememberState')) { + return true; + } + + const children = node.children?.children; + if (!children) { + return false; + } + + for (const child of children) { + if (hasRememberStorage(child)) { + return true; + } + } + + return false; +} + +describe('remember', () => { + it('does not allocate remember storage when unused', () => { + const renderer = makeRenderer(); + const rootPrototype = makeNodePrototype('view'); + + renderer.begin(); + renderer.beginElement(rootPrototype); + renderer.endElement(); + renderer.end(); + + expect(hasRememberStorage((renderer as any).nodeTree)).toBe(false); + }); + + it('persists values with no keys across renders of the same VirtualNode', () => { + const renderer = makeRenderer(); + const rootPrototype = makeNodePrototype('view'); + let factoryCalls = 0; + let renderedValue: RememberedValue | undefined; + + function render() { + renderer.begin(); + renderer.beginElement(rootPrototype); + renderedValue = remember(() => ({ id: ++factoryCalls })); + renderer.endElement(); + renderer.end(); + } + + render(); + const firstValue = renderedValue; + render(); + + expect(renderedValue).toBe(firstValue); + expect(factoryCalls).toBe(1); + }); + + it('compares keys with Object.is', () => { + const renderer = makeRenderer(); + const rootPrototype = makeNodePrototype('view'); + let factoryCalls = 0; + let renderedValue = 0; + + function render(key: unknown) { + renderer.begin(); + renderer.beginElement(rootPrototype); + renderedValue = remember(() => ++factoryCalls, key); + renderer.endElement(); + renderer.end(); + } + + render(NaN); + expect(renderedValue).toBe(1); + + render(NaN); + expect(renderedValue).toBe(1); + expect(factoryCalls).toBe(1); + + render(0); + expect(renderedValue).toBe(2); + + render(-0); + expect(renderedValue).toBe(3); + expect(factoryCalls).toBe(3); + }); + + it('keeps multiple calls in the same VirtualNode independent', () => { + const renderer = makeRenderer(); + const rootPrototype = makeNodePrototype('view'); + let firstFactoryCalls = 0; + let secondFactoryCalls = 0; + let firstValue: RememberedValue | undefined; + let secondValue: RememberedValue | undefined; + + function render() { + renderer.begin(); + renderer.beginElement(rootPrototype); + firstValue = remember(() => ({ id: ++firstFactoryCalls }), 'same-key'); + secondValue = remember(() => ({ id: ++secondFactoryCalls }), 'same-key'); + renderer.endElement(); + renderer.end(); + } + + render(); + const initialFirstValue = firstValue; + const initialSecondValue = secondValue; + render(); + + expect(firstValue).toBe(initialFirstValue); + expect(secondValue).toBe(initialSecondValue); + expect(firstValue).not.toBe(secondValue); + expect(firstFactoryCalls).toBe(1); + expect(secondFactoryCalls).toBe(1); + }); + + it('scopes remembered values to keyed VirtualNodes across sibling reorders', () => { + const renderer = makeRenderer(); + const rootPrototype = makeNodePrototype('view'); + const itemPrototype = makeNodePrototype('label'); + let nextId = 0; + + function render(keys: string[]): { [key: string]: RememberedValue } { + const values: { [key: string]: RememberedValue } = {}; + + renderer.begin(); + renderer.beginElement(rootPrototype); + for (const key of keys) { + renderer.beginElement(itemPrototype, key); + values[key] = remember(() => ({ id: ++nextId, itemKey: key })); + renderer.endElement(); + } + renderer.endElement(); + renderer.end(); + + return values; + } + + const firstRender = render(['one', 'two', 'three']); + const secondRender = render(['three', 'one', 'two']); + + expect(secondRender.one).toBe(firstRender.one); + expect(secondRender.two).toBe(firstRender.two); + expect(secondRender.three).toBe(firstRender.three); + expect(nextId).toBe(3); + }); + + it('forgets values when a VirtualNode is removed', () => { + const renderer = makeRenderer(); + const rootPrototype = makeNodePrototype('view'); + const itemPrototype = makeNodePrototype('label'); + let factoryCalls = 0; + + function render(showItem: boolean): RememberedValue | undefined { + let value: RememberedValue | undefined; + + renderer.begin(); + renderer.beginElement(rootPrototype); + if (showItem) { + renderer.beginElement(itemPrototype, 'item'); + value = remember(() => ({ id: ++factoryCalls })); + renderer.endElement(); + } + renderer.endElement(); + renderer.end(); + + return value; + } + + const firstValue = render(true); + render(false); + const secondValue = render(true); + + expect(firstValue).toBeDefined(); + expect(secondValue).toBeDefined(); + expect(secondValue).not.toBe(firstValue); + expect(factoryCalls).toBe(2); + }); + + it('throws when called outside a render', () => { + let error: unknown; + try { + remember(() => 1); + } catch (err: unknown) { + error = err; + } + + expect(String(error)).toContain('Cannot call this outside of a onRender callback'); + }); +}); diff --git a/src/valdi_modules/src/valdi/valdi_test/test/Renderer.spec.ts b/src/valdi_modules/src/valdi/valdi_test/test/Renderer.spec.ts index de20cd7e..cd8ee352 100644 --- a/src/valdi_modules/src/valdi/valdi_test/test/Renderer.spec.ts +++ b/src/valdi_modules/src/valdi/valdi_test/test/Renderer.spec.ts @@ -17,6 +17,8 @@ import { PropertyList } from 'valdi_tsx/src/PropertyList'; import { StringMap } from 'coreutils/src/StringMap'; import { RawRenderRequestEntryType, RendererTestDelegate } from './RendererTestDelegate'; +const createdRenderers: Renderer[] = []; + function makeRenderer( delegate: IRendererDelegate, allowedRootElementTypes?: string[], @@ -24,6 +26,7 @@ function makeRenderer( useTopDownMoveOrder?: boolean, ): Renderer { const renderer = new Renderer('', allowedRootElementTypes, delegate, useTopDownMoveOrder); + createdRenderers.push(renderer); // return renderer; if (disableProxy) { @@ -52,6 +55,12 @@ function makeRenderer( }); } +function cleanupRendererAfterExpectedError(renderer: Renderer): void { + if (renderer.began) { + renderer.doEnd(); + } +} + function sanitize(text: string): string { const outLines: string[] = []; @@ -111,6 +120,16 @@ class TestComponent implements IComponent { } describe('Renderer', () => { + // Renderer keeps the active renderer in module-level state. Some specs intentionally + // throw during a render before reaching renderer.end(), so clean up after every + // spec to keep randomized test order from leaking active renderer state. + afterEach(() => { + for (const renderer of createdRenderers) { + cleanupRendererAfterExpectedError(renderer); + } + createdRenderers.length = 0; + }); + it('doesnt render on empty body', () => { const output = new RendererTestDelegate(); const renderer = makeRenderer(output); @@ -3038,11 +3057,15 @@ describe('Renderer', () => { function render(renderSlot: () => void) { renderer.begin(); - renderer.beginComponent(SlottedComponent, componentPrototype); + try { + renderer.beginComponent(SlottedComponent, componentPrototype); - renderSlot(); - renderer.endComponent(); - renderer.end(); + renderSlot(); + renderer.endComponent(); + renderer.end(); + } finally { + cleanupRendererAfterExpectedError(renderer); + } } render(() => {}); @@ -5327,7 +5350,7 @@ Begin render Begin RootComponent (key: __root) Begin ChildComponent (key: __child) End ChildComponent - Begin ChildComponent (key: __child2) + Begin ChildComponent (key: __child-2) End ChildComponent End RootComponent End render @@ -5357,7 +5380,7 @@ Begin render Begin ChildComponent (key: __child) ViewModel property 'name' changed End ChildComponent - Begin ChildComponent (key: __child2) + Begin ChildComponent (key: __child-2) Bypass render End ChildComponent End RootComponent diff --git a/valdi/src/valdi/runtime/Attributes/ViewNodeAttributesApplier.cpp b/valdi/src/valdi/runtime/Attributes/ViewNodeAttributesApplier.cpp index 047351c6..ac4cbef3 100644 --- a/valdi/src/valdi/runtime/Attributes/ViewNodeAttributesApplier.cpp +++ b/valdi/src/valdi/runtime/Attributes/ViewNodeAttributesApplier.cpp @@ -488,6 +488,16 @@ void ViewNodeAttributesApplier::updateAttributeHandlers() { } }); } + + auto dirtyIt = _dirtyCompositeAttributes.begin(); + while (dirtyIt != _dirtyCompositeAttributes.end()) { + const auto* handler = _boundAttributes->getAttributeHandlerForId(dirtyIt->first); + if (handler == nullptr || handler->getCompositeAttribute() == nullptr) { + dirtyIt = _dirtyCompositeAttributes.erase(dirtyIt); + } else { + ++dirtyIt; + } + } } void ViewNodeAttributesApplier::destroy() { diff --git a/valdi/test/runtime/ViewNode_tests.cpp b/valdi/test/runtime/ViewNode_tests.cpp index f4f12bd8..5e1e76d8 100644 --- a/valdi/test/runtime/ViewNode_tests.cpp +++ b/valdi/test/runtime/ViewNode_tests.cpp @@ -17,6 +17,22 @@ void assertAllFlagsAreUpToDate(ViewNode* viewNode) { } } +TEST(ViewNode, classChangePrunesDirtyCompositeAttributes) { + ViewNodeTestsDependencies utils; + utils.getViewManager().setRegisterCustomAttributes(true); + + auto viewNode = utils.createNode("UIRectangleView"); + utils.setViewNodeAttribute(viewNode, "left", Value(10.0)); + + ASSERT_TRUE(viewNode->getAttributesApplier().needsFlush()); + + // Platform class attributes use this same setter in production; this fixture has no ViewManagerContext. + viewNode->setViewFactory(utils.getViewTransactionScope(), utils.getViewFactory("SCValdiLabel")); + + EXPECT_FALSE(viewNode->getAttributesApplier().needsFlush()); + viewNode->getAttributesApplier().flush(utils.getViewTransactionScope()); +} + TEST(ViewNode, canInsertChildren) { ViewNodeTestsDependencies utils; diff --git a/valdi/test/utils/ViewNodeTestsUtils.cpp b/valdi/test/utils/ViewNodeTestsUtils.cpp index ebbddd6e..1d876157 100644 --- a/valdi/test/utils/ViewNodeTestsUtils.cpp +++ b/valdi/test/utils/ViewNodeTestsUtils.cpp @@ -87,6 +87,11 @@ StandaloneViewManager& ViewNodeTestsDependencies::getViewManager() { return _viewManager; } +Ref ViewNodeTestsDependencies::getViewFactory(const char* viewClassName) { + auto className = StringCache::getGlobal().makeStringFromLiteral(viewClassName); + return _viewFactories->getViewFactory(className); +} + void ViewNodeTestsDependencies::setViewNodeAttribute(const Ref& viewNode, const char* attribute, const Value& attributeValue) { diff --git a/valdi/test/utils/ViewNodeTestsUtils.hpp b/valdi/test/utils/ViewNodeTestsUtils.hpp index c7024fa9..41d98ec3 100644 --- a/valdi/test/utils/ViewNodeTestsUtils.hpp +++ b/valdi/test/utils/ViewNodeTestsUtils.hpp @@ -39,6 +39,7 @@ class ViewNodeTestsDependencies { void disableUpdates(); StandaloneViewManager& getViewManager(); + Ref getViewFactory(const char* viewClassName); void setViewNodeAttribute(const Ref& viewNode, const char* attribute, const Value& attributeValue);