diff --git a/.changeset/new-avocados-walk.md b/.changeset/new-avocados-walk.md new file mode 100644 index 0000000..88cf689 --- /dev/null +++ b/.changeset/new-avocados-walk.md @@ -0,0 +1,5 @@ +--- +"@marko/tags-api-preview": patch +--- + +improve member expression caching diff --git a/src/__tests__/fixtures/const/__snapshots__/const-state-derived-from-prototype-function/node.render.expected.html b/src/__tests__/fixtures/const/__snapshots__/const-state-derived-from-prototype-function/node.render.expected.html new file mode 100644 index 0000000..c1c800b --- /dev/null +++ b/src/__tests__/fixtures/const/__snapshots__/const-state-derived-from-prototype-function/node.render.expected.html @@ -0,0 +1,6 @@ +

+ 9 string length = 1 +

+ \ No newline at end of file diff --git a/src/__tests__/fixtures/const/__snapshots__/const-state-derived-from-prototype-function/web.render.expected.html b/src/__tests__/fixtures/const/__snapshots__/const-state-derived-from-prototype-function/web.render.expected.html new file mode 100644 index 0000000..c1c800b --- /dev/null +++ b/src/__tests__/fixtures/const/__snapshots__/const-state-derived-from-prototype-function/web.render.expected.html @@ -0,0 +1,6 @@ +

+ 9 string length = 1 +

+ \ No newline at end of file diff --git a/src/__tests__/fixtures/const/__snapshots__/const-state-derived-from-prototype-function/web.step-0.expected.html b/src/__tests__/fixtures/const/__snapshots__/const-state-derived-from-prototype-function/web.step-0.expected.html new file mode 100644 index 0000000..3b6463b --- /dev/null +++ b/src/__tests__/fixtures/const/__snapshots__/const-state-derived-from-prototype-function/web.step-0.expected.html @@ -0,0 +1,6 @@ +

+ 10 string length = 2 +

+ \ No newline at end of file diff --git a/src/__tests__/fixtures/const/index.test.ts b/src/__tests__/fixtures/const/index.test.ts index e5dc94d..5675adc 100644 --- a/src/__tests__/fixtures/const/index.test.ts +++ b/src/__tests__/fixtures/const/index.test.ts @@ -21,6 +21,16 @@ describe( ]) ); +describe( + " state derived from prototype function", + fixture("./templates/state-derived-prototype-function.marko", [ + { value: 1 }, + async ({ screen, fireEvent }) => { + await fireEvent.click(screen.getByText("increment")); + }, + ]) +); + describe( " assignment error", fixture("./templates/error-assignment.marko") diff --git a/src/__tests__/fixtures/const/templates/state-derived-prototype-function.marko b/src/__tests__/fixtures/const/templates/state-derived-prototype-function.marko new file mode 100644 index 0000000..3c31d4c --- /dev/null +++ b/src/__tests__/fixtures/const/templates/state-derived-prototype-function.marko @@ -0,0 +1,4 @@ + + +

${value} string length = ${length}

+ diff --git a/src/__tests__/fixtures/misc/cached-conditional-value/index.test.ts b/src/__tests__/fixtures/misc/cached-conditional-value/index.test.ts deleted file mode 100644 index 6f2cc9e..0000000 --- a/src/__tests__/fixtures/misc/cached-conditional-value/index.test.ts +++ /dev/null @@ -1,20 +0,0 @@ -import fixture, { FixtureHelpers } from "../../../fixture"; -const increment = click("Increment"); -const toggle = click("Toggle"); - -describe( - "cached conditional value", - fixture("./templates/basic.marko", [ - {}, - increment, - toggle, - toggle, - increment, - toggle, - ]) -); - -function click(text: string) { - return async ({ fireEvent, screen }: FixtureHelpers) => - await fireEvent.click(screen.getByText(text)); -} diff --git a/src/__tests__/fixtures/misc/cached-conditional-value/__snapshots__/cached-conditional-value/node.render.expected.html b/src/__tests__/fixtures/misc/caching/__snapshots__/cached/conditional-value/node.render.expected.html similarity index 100% rename from src/__tests__/fixtures/misc/cached-conditional-value/__snapshots__/cached-conditional-value/node.render.expected.html rename to src/__tests__/fixtures/misc/caching/__snapshots__/cached/conditional-value/node.render.expected.html diff --git a/src/__tests__/fixtures/misc/cached-conditional-value/__snapshots__/cached-conditional-value/web.render.expected.html b/src/__tests__/fixtures/misc/caching/__snapshots__/cached/conditional-value/web.render.expected.html similarity index 100% rename from src/__tests__/fixtures/misc/cached-conditional-value/__snapshots__/cached-conditional-value/web.render.expected.html rename to src/__tests__/fixtures/misc/caching/__snapshots__/cached/conditional-value/web.render.expected.html diff --git a/src/__tests__/fixtures/misc/cached-conditional-value/__snapshots__/cached-conditional-value/web.step-0.expected.html b/src/__tests__/fixtures/misc/caching/__snapshots__/cached/conditional-value/web.step-0.expected.html similarity index 100% rename from src/__tests__/fixtures/misc/cached-conditional-value/__snapshots__/cached-conditional-value/web.step-0.expected.html rename to src/__tests__/fixtures/misc/caching/__snapshots__/cached/conditional-value/web.step-0.expected.html diff --git a/src/__tests__/fixtures/misc/cached-conditional-value/__snapshots__/cached-conditional-value/web.step-1.expected.html b/src/__tests__/fixtures/misc/caching/__snapshots__/cached/conditional-value/web.step-1.expected.html similarity index 100% rename from src/__tests__/fixtures/misc/cached-conditional-value/__snapshots__/cached-conditional-value/web.step-1.expected.html rename to src/__tests__/fixtures/misc/caching/__snapshots__/cached/conditional-value/web.step-1.expected.html diff --git a/src/__tests__/fixtures/misc/cached-conditional-value/__snapshots__/cached-conditional-value/web.step-2.expected.html b/src/__tests__/fixtures/misc/caching/__snapshots__/cached/conditional-value/web.step-2.expected.html similarity index 100% rename from src/__tests__/fixtures/misc/cached-conditional-value/__snapshots__/cached-conditional-value/web.step-2.expected.html rename to src/__tests__/fixtures/misc/caching/__snapshots__/cached/conditional-value/web.step-2.expected.html diff --git a/src/__tests__/fixtures/misc/cached-conditional-value/__snapshots__/cached-conditional-value/web.step-3.expected.html b/src/__tests__/fixtures/misc/caching/__snapshots__/cached/conditional-value/web.step-3.expected.html similarity index 100% rename from src/__tests__/fixtures/misc/cached-conditional-value/__snapshots__/cached-conditional-value/web.step-3.expected.html rename to src/__tests__/fixtures/misc/caching/__snapshots__/cached/conditional-value/web.step-3.expected.html diff --git a/src/__tests__/fixtures/misc/cached-conditional-value/__snapshots__/cached-conditional-value/web.step-4.expected.html b/src/__tests__/fixtures/misc/caching/__snapshots__/cached/conditional-value/web.step-4.expected.html similarity index 100% rename from src/__tests__/fixtures/misc/cached-conditional-value/__snapshots__/cached-conditional-value/web.step-4.expected.html rename to src/__tests__/fixtures/misc/caching/__snapshots__/cached/conditional-value/web.step-4.expected.html diff --git a/src/__tests__/fixtures/misc/caching/__snapshots__/cached/function-call/node.render.expected.html b/src/__tests__/fixtures/misc/caching/__snapshots__/cached/function-call/node.render.expected.html new file mode 100644 index 0000000..98bb062 --- /dev/null +++ b/src/__tests__/fixtures/misc/caching/__snapshots__/cached/function-call/node.render.expected.html @@ -0,0 +1,3 @@ + \ No newline at end of file diff --git a/src/__tests__/fixtures/misc/caching/__snapshots__/cached/function-call/web.render.expected.html b/src/__tests__/fixtures/misc/caching/__snapshots__/cached/function-call/web.render.expected.html new file mode 100644 index 0000000..98bb062 --- /dev/null +++ b/src/__tests__/fixtures/misc/caching/__snapshots__/cached/function-call/web.render.expected.html @@ -0,0 +1,3 @@ + \ No newline at end of file diff --git a/src/__tests__/fixtures/misc/caching/__snapshots__/cached/function-call/web.step-0.expected.html b/src/__tests__/fixtures/misc/caching/__snapshots__/cached/function-call/web.step-0.expected.html new file mode 100644 index 0000000..98bb062 --- /dev/null +++ b/src/__tests__/fixtures/misc/caching/__snapshots__/cached/function-call/web.step-0.expected.html @@ -0,0 +1,3 @@ + \ No newline at end of file diff --git a/src/__tests__/fixtures/misc/caching/index.test.ts b/src/__tests__/fixtures/misc/caching/index.test.ts new file mode 100644 index 0000000..9216077 --- /dev/null +++ b/src/__tests__/fixtures/misc/caching/index.test.ts @@ -0,0 +1,51 @@ +import { resetHistory, spy } from "sinon"; +import fixture, { FixtureHelpers } from "../../../fixture"; +const increment = click("Increment"); +const toggle = click("Toggle"); + +const onRender = spy(); + +describe("cached", () => { + beforeEach(() => resetHistory()); + + describe( + "conditional value", + fixture("./templates/conditional-value.marko", [ + {}, + increment, + toggle, + toggle, + increment, + toggle, + ]) + ); + + describe( + "function call", + fixture("./templates/function-call.marko", [ + { fn: onRender }, + async ({ expect, screen, fireEvent, rerender }) => { + expect(onRender).calledOnce; + resetHistory(); + expect(onRender).not.called; + + await fireEvent.click(screen.getByText("increment")); + expect(onRender).not.called; + resetHistory(); + + await rerender(); + expect(onRender).not.called; + resetHistory(); + + await rerender({ fn: () => onRender() }); + expect(onRender).calledOnce; + resetHistory(); + }, + ]) + ); +}); + +function click(text: string) { + return async ({ fireEvent, screen }: FixtureHelpers) => + await fireEvent.click(screen.getByText(text)); +} diff --git a/src/__tests__/fixtures/misc/cached-conditional-value/templates/basic.marko b/src/__tests__/fixtures/misc/caching/templates/conditional-value.marko similarity index 100% rename from src/__tests__/fixtures/misc/cached-conditional-value/templates/basic.marko rename to src/__tests__/fixtures/misc/caching/templates/conditional-value.marko diff --git a/src/__tests__/fixtures/misc/caching/templates/function-call.marko b/src/__tests__/fixtures/misc/caching/templates/function-call.marko new file mode 100644 index 0000000..056b4b9 --- /dev/null +++ b/src/__tests__/fixtures/misc/caching/templates/function-call.marko @@ -0,0 +1,3 @@ + + + \ No newline at end of file diff --git a/src/transform/cached-values/transform.ts b/src/transform/cached-values/transform.ts index 0d4b56b..d97e851 100644 --- a/src/transform/cached-values/transform.ts +++ b/src/transform/cached-values/transform.ts @@ -10,12 +10,19 @@ interface Deps { } interface DepsVisitorState { + parent: t.Node; + node: t.Node; parentTag: t.NodePath; deps: Deps | undefined; shouldCache: boolean; } const depsVisitor = { + enter(path, state) { + if (path.parent === state.parent && path.node !== state.node) { + path.skip(); + } + }, Function(_, state) { state.shouldCache = true; }, @@ -55,22 +62,25 @@ const depsVisitor = { const curDeps = deps[name]; if (curDeps === true) return; - if ( - parent.isMemberExpression() && - (!parent.node.computed || - isStringOrNumericLiteral(parent.node.property.type)) - ) { - const prop = parent.node.property as - | t.Identifier - | t.StringLiteral - | t.NumericLiteral; - parent = parent.parentPath; - deps = curDeps || (deps[name] = {} as Deps); - name = prop.type === "Identifier" ? prop.name : prop.value + ""; - } else { - deps[name] = true; - return; + if (parent.isMemberExpression()) { + const literalName = getPropertyNameLiteral(parent.node); + if ( + literalName !== undefined && + !( + parent.parentPath.isCallExpression() && + parent.parentPath.node.callee === parent.node && + !isEventHandlerName(literalName) + ) + ) { + parent = parent.parentPath; + deps = curDeps || (deps[name] = {} as Deps); + name = literalName; + continue; + } } + + deps[name] = true; + return; // eslint-disable-next-line no-constant-condition } while (true); } @@ -114,19 +124,22 @@ export default { function cacheExprIfNeeded( parentTag: t.NodePath, - expr: t.NodePath + exprPath: t.NodePath ) { + const parentPath = exprPath.parentPath!; const state: DepsVisitorState = { parentTag, + node: exprPath.node, + parent: parentPath.node, deps: undefined, shouldCache: false, }; - expr.traverse(depsVisitor, state); + parentPath.traverse(depsVisitor, state); if (state.shouldCache) { - const { file } = expr.hub; + const { file } = exprPath.hub; const { component } = ensureLifecycle(parentTag)!; - expr + exprPath .replaceWith( t.callExpression( importRuntimeNamed(file, "transform/cached-values", "cache"), @@ -140,7 +153,7 @@ function cacheExprIfNeeded( t.arrayExpression(state.deps ? toDepsArray(state.deps) : []), ] ), - expr.node + exprPath.node ), ] ) @@ -174,6 +187,19 @@ function* getDefaultExpressions( } } +function getPropertyNameLiteral(memberExpression: t.MemberExpression) { + if (memberExpression.computed) { + if (isStringOrNumericLiteral(memberExpression.property.type)) { + return ( + (memberExpression.property as t.StringLiteral | t.NumericLiteral) + .value + "" + ); + } + } else if (memberExpression.property.type === "Identifier") { + return memberExpression.property.name; + } +} + function isStringOrNumericLiteral(type: string) { switch (type) { case "StringLiteral": @@ -184,6 +210,10 @@ function isStringOrNumericLiteral(type: string) { } } +function isEventHandlerName(name: string) { + return /^on[A-Z]/.test(name); +} + function toDepsArray( deps: Deps, object?: t.Expression,