From 4e2ef92779bd4e55583bd3be1d88399cb4c883bb Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Fri, 31 Mar 2023 12:32:45 -0700 Subject: [PATCH] Ensure children are not independently memod MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a Meta-ism, but adding it for now to unblock. We special-case the `` element for translation purposes, and have a transform that requires the children of this element to be a limited subset of nodes. Notably, any dynamic translation values must appear as `` children — we disallow identifiers as children of `` nodes. This PR adds a new pass which finds `` nodes and ensures their immediate operands are not independently memoized. Note that this still allows the values of `` to be independently memoized, as demonstrated in the unit test. --- compiler/forget/src/CompilerPipeline.ts | 10 ++- .../MemoizeFbtOperandsInSameScope.ts | 72 +++++++++++++++++++ compiler/forget/src/ReactiveScopes/index.ts | 1 + .../fbt-params-complex-param-value.expect.md | 46 ++++++++++++ .../fbt-params-complex-param-value.js | 7 ++ .../fixtures/compiler/fbt-params.expect.md | 37 ++++++++++ .../__tests__/fixtures/compiler/fbt-params.js | 7 ++ compiler/forget/src/index.ts | 2 + compiler/forget/yarn.lock | 47 ++++++------ 9 files changed, 208 insertions(+), 21 deletions(-) create mode 100644 compiler/forget/src/ReactiveScopes/MemoizeFbtOperandsInSameScope.ts create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/fbt-params-complex-param-value.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/fbt-params-complex-param-value.js create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/fbt-params.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/fbt-params.js diff --git a/compiler/forget/src/CompilerPipeline.ts b/compiler/forget/src/CompilerPipeline.ts index 80fb6dc299..a7e5db4da1 100644 --- a/compiler/forget/src/CompilerPipeline.ts +++ b/compiler/forget/src/CompilerPipeline.ts @@ -12,7 +12,7 @@ import { mergeConsecutiveBlocks, ReactiveFunction, } from "./HIR"; -import { EnvironmentConfig, Environment } from "./HIR/Environment"; +import { Environment, EnvironmentConfig } from "./HIR/Environment"; import { validateConsistentIdentifiers } from "./HIR/ValidateConsistentIdentifiers"; import { analyseFunctions, @@ -29,6 +29,7 @@ import { flattenReactiveLoops, flattenScopesWithHooks, inferReactiveScopeVariables, + memoizeFbtOperandsInSameScope, mergeOverlappingReactiveScopes, promoteUsedTemporaries, propagateScopeDependencies, @@ -104,6 +105,13 @@ export function* run( value: reactiveFunction, }); + memoizeFbtOperandsInSameScope(reactiveFunction); + yield log({ + kind: "reactive", + name: "MemoizeFbtOperandsInSameScope", + value: reactiveFunction, + }); + alignReactiveScopesToBlockScopes(reactiveFunction); yield log({ kind: "reactive", diff --git a/compiler/forget/src/ReactiveScopes/MemoizeFbtOperandsInSameScope.ts b/compiler/forget/src/ReactiveScopes/MemoizeFbtOperandsInSameScope.ts new file mode 100644 index 0000000000..c91df8874d --- /dev/null +++ b/compiler/forget/src/ReactiveScopes/MemoizeFbtOperandsInSameScope.ts @@ -0,0 +1,72 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import { + IdentifierId, + makeInstructionId, + ReactiveFunction, + ReactiveInstruction, +} from "../HIR"; +import { eachInstructionValueOperand } from "../HIR/visitors"; +import { ReactiveFunctionVisitor, visitReactiveFunction } from "./visitors"; + +/** + * This is a Meta-ism. We special-case the `` element for translation purposes, + * and have a transform that requires the children of this element to be a limited + * subset of nodes. Notably, any dynamic translation values must appear as + * `` children — we disallow identifiers as children of `` nodes. + * + * This PR adds a new pass which finds `` nodes and ensures their immediate + * operands are not independently memoized. Note that this still allows the values + * of `` to be independently memoized + */ +export function memoizeFbtOperandsInSameScope(fn: ReactiveFunction): void { + visitReactiveFunction(fn, new Transform(), undefined); +} + +class Transform extends ReactiveFunctionVisitor { + fbtTags: Set = new Set(); + + override visitInstruction( + instruction: ReactiveInstruction, + _state: void + ): void { + const { lvalue, value } = instruction; + if (lvalue === null) { + return; + } + if ( + value.kind === "Primitive" && + typeof value.value === "string" && + value.value === "fbt" + ) { + // We don't distinguish between tag names and strings, so record + // all `fbt` string literals in case they are used as a jsx tag. + this.fbtTags.add(lvalue.identifier.id); + } else if ( + value.kind === "JsxExpression" && + this.fbtTags.has(value.tag.identifier.id) + ) { + // if the JSX element's tag was `fbt`, mark all its operands + // to ensure that they end up in the same scope as the jsx element + // itself. + for (const operand of eachInstructionValueOperand(value)) { + operand.identifier.scope = lvalue.identifier.scope; + operand.identifier.mutableRange.end = + lvalue.identifier.mutableRange.end; + + // Expand the jsx element's range to account for its operands + lvalue.identifier.mutableRange.start = makeInstructionId( + Math.min( + lvalue.identifier.mutableRange.start, + operand.identifier.mutableRange.start + ) + ); + } + } + } +} diff --git a/compiler/forget/src/ReactiveScopes/index.ts b/compiler/forget/src/ReactiveScopes/index.ts index ad5b55756c..7c553f2db5 100644 --- a/compiler/forget/src/ReactiveScopes/index.ts +++ b/compiler/forget/src/ReactiveScopes/index.ts @@ -12,6 +12,7 @@ export { codegenReactiveFunction } from "./CodegenReactiveFunction"; export { flattenReactiveLoops } from "./FlattenReactiveLoops"; export { flattenScopesWithHooks } from "./FlattenScopesWithHooks"; export { inferReactiveScopeVariables } from "./InferReactiveScopeVariables"; +export { memoizeFbtOperandsInSameScope } from "./MemoizeFbtOperandsInSameScope"; export { mergeOverlappingReactiveScopes } from "./MergeOverlappingReactiveScopes"; export { printReactiveFunction } from "./PrintReactiveFunction"; export { promoteUsedTemporaries } from "./PromoteUsedTemporaries"; diff --git a/compiler/forget/src/__tests__/fixtures/compiler/fbt-params-complex-param-value.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/fbt-params-complex-param-value.expect.md new file mode 100644 index 0000000000..6b23df4a00 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/fbt-params-complex-param-value.expect.md @@ -0,0 +1,46 @@ + +## Input + +```javascript +function Component(props) { + return ( + + Hello {capitalize(props.name)} + + ); +} + +``` + +## Code + +```javascript +function Component(props) { + const $ = React.unstable_useMemoCache(4); + const c_0 = $[0] !== props.name; + let t1; + if (c_0) { + const c_2 = $[2] !== props.name; + let t0; + if (c_2) { + t0 = capitalize(props.name); + $[2] = props.name; + $[3] = t0; + } else { + t0 = $[3]; + } + t1 = ( + + Hello {{t0}} + + ); + $[0] = props.name; + $[1] = t1; + } else { + t1 = $[1]; + } + return t1; +} + +``` + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/compiler/fbt-params-complex-param-value.js b/compiler/forget/src/__tests__/fixtures/compiler/fbt-params-complex-param-value.js new file mode 100644 index 0000000000..d349b67bff --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/fbt-params-complex-param-value.js @@ -0,0 +1,7 @@ +function Component(props) { + return ( + + Hello {capitalize(props.name)} + + ); +} diff --git a/compiler/forget/src/__tests__/fixtures/compiler/fbt-params.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/fbt-params.expect.md new file mode 100644 index 0000000000..2440cbe8c7 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/fbt-params.expect.md @@ -0,0 +1,37 @@ + +## Input + +```javascript +function Component(props) { + return ( + + Hello {props.name} + + ); +} + +``` + +## Code + +```javascript +function Component(props) { + const $ = React.unstable_useMemoCache(2); + const c_0 = $[0] !== props.name; + let t0; + if (c_0) { + t0 = ( + + Hello {{props.name}} + + ); + $[0] = props.name; + $[1] = t0; + } else { + t0 = $[1]; + } + return t0; +} + +``` + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/compiler/fbt-params.js b/compiler/forget/src/__tests__/fixtures/compiler/fbt-params.js new file mode 100644 index 0000000000..b02a983952 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/fbt-params.js @@ -0,0 +1,7 @@ +function Component(props) { + return ( + + Hello {props.name} + + ); +} diff --git a/compiler/forget/src/index.ts b/compiler/forget/src/index.ts index fb58b437ea..fb453a91fe 100644 --- a/compiler/forget/src/index.ts +++ b/compiler/forget/src/index.ts @@ -18,5 +18,7 @@ declare global { let __DEV__: boolean | null | undefined; } +console.log("loading Forget!!!"); + import ReactForgetBabelPlugin from "./Babel/BabelPlugin"; export default ReactForgetBabelPlugin; diff --git a/compiler/forget/yarn.lock b/compiler/forget/yarn.lock index 8001987f45..c3391a73ae 100644 --- a/compiler/forget/yarn.lock +++ b/compiler/forget/yarn.lock @@ -17,6 +17,13 @@ dependencies: "@babel/highlight" "^7.18.6" +"@babel/code-frame@^7.21.4": + version "7.21.4" + resolved "https://registry.yarnpkg.com/@babel/code-frame/-/code-frame-7.21.4.tgz#d0fa9e4413aca81f2b23b9442797bda1826edb39" + integrity sha512-LYvhNKfwWSPpocw8GI7gpK2nq3HSDuEPC/uSYaALSJu9xjsalaaYFOq0Pwt5KmVqwEbZlDu81aLXwBOmD/Fv9g== + dependencies: + "@babel/highlight" "^7.18.6" + "@babel/compat-data@^7.19.1": version "7.19.1" resolved "https://registry.yarnpkg.com/@babel/compat-data/-/compat-data-7.19.1.tgz#72d647b4ff6a4f82878d184613353af1dd0290f9" @@ -83,12 +90,12 @@ "@jridgewell/gen-mapping" "^0.3.2" jsesc "^2.5.1" -"@babel/generator@^7.21.3": - version "7.21.3" - resolved "https://registry.yarnpkg.com/@babel/generator/-/generator-7.21.3.tgz#232359d0874b392df04045d72ce2fd9bb5045fce" - integrity sha512-QS3iR1GYC/YGUnW7IdggFeN5c1poPUurnGttOV/bZgPGV+izC/D8HnD6DLwod0fsatNyVn1G3EVWMYIF0nHbeA== +"@babel/generator@^7.21.4": + version "7.21.4" + resolved "https://registry.yarnpkg.com/@babel/generator/-/generator-7.21.4.tgz#64a94b7448989f421f919d5239ef553b37bb26bc" + integrity sha512-NieM3pVIYW2SwGzKoqfPrQsf4xGs9M9AIG3ThppsSRmO+m7eQhmI6amajKMUeIO37wFfsvnvcxQFx6x6iqxDnA== dependencies: - "@babel/types" "^7.21.3" + "@babel/types" "^7.21.4" "@jridgewell/gen-mapping" "^0.3.2" "@jridgewell/trace-mapping" "^0.3.17" jsesc "^2.5.1" @@ -274,10 +281,10 @@ resolved "https://registry.yarnpkg.com/@babel/parser/-/parser-7.21.2.tgz#dacafadfc6d7654c3051a66d6fe55b6cb2f2a0b3" integrity sha512-URpaIJQwEkEC2T9Kn+Ai6Xe/02iNaVCuT/PtoRz3GPVJVDpPd7mLo+VddTbhCRU9TXqW5mSrQfXZyi8kDKOVpQ== -"@babel/parser@^7.21.3": - version "7.21.3" - resolved "https://registry.yarnpkg.com/@babel/parser/-/parser-7.21.3.tgz#1d285d67a19162ff9daa358d4cb41d50c06220b3" - integrity sha512-lobG0d7aOfQRXh8AyklEAgZGvA4FShxo6xQbUrrT/cNBPUdIDojlokwJsQyCC/eKia7ifqM0yP+2DRZ4WKw2RQ== +"@babel/parser@^7.21.4": + version "7.21.4" + resolved "https://registry.yarnpkg.com/@babel/parser/-/parser-7.21.4.tgz#94003fdfc520bbe2875d4ae557b43ddb6d880f17" + integrity sha512-alVJj7k7zIxqBZ7BTRhz0IqJFxW1VJbm6N8JbcYhQ186df9ZBPbZBmWSqAMXwHGsCJdYks7z/voa3ibiS5bCIw== "@babel/plugin-syntax-async-generators@^7.8.4": version "7.8.4" @@ -507,18 +514,18 @@ lodash "^4.17.10" "@babel/traverse@^7.19.1": - version "7.21.3" - resolved "https://registry.yarnpkg.com/@babel/traverse/-/traverse-7.21.3.tgz#4747c5e7903d224be71f90788b06798331896f67" - integrity sha512-XLyopNeaTancVitYZe2MlUEvgKb6YVVPXzofHgqHijCImG33b/uTurMS488ht/Hbsb2XK3U2BnSTxKVNGV3nGQ== + version "7.21.4" + resolved "https://registry.yarnpkg.com/@babel/traverse/-/traverse-7.21.4.tgz#a836aca7b116634e97a6ed99976236b3282c9d36" + integrity sha512-eyKrRHKdyZxqDm+fV1iqL9UAHMoIg0nDaGqfIOd8rKH17m5snv7Gn4qgjBoFfLz9APvjFU/ICT00NVCv1Epp8Q== dependencies: - "@babel/code-frame" "^7.18.6" - "@babel/generator" "^7.21.3" + "@babel/code-frame" "^7.21.4" + "@babel/generator" "^7.21.4" "@babel/helper-environment-visitor" "^7.18.9" "@babel/helper-function-name" "^7.21.0" "@babel/helper-hoist-variables" "^7.18.6" "@babel/helper-split-export-declaration" "^7.18.6" - "@babel/parser" "^7.21.3" - "@babel/types" "^7.21.3" + "@babel/parser" "^7.21.4" + "@babel/types" "^7.21.4" debug "^4.1.0" globals "^11.1.0" @@ -575,10 +582,10 @@ "@babel/helper-validator-identifier" "^7.19.1" to-fast-properties "^2.0.0" -"@babel/types@^7.21.3": - version "7.21.3" - resolved "https://registry.yarnpkg.com/@babel/types/-/types-7.21.3.tgz#4865a5357ce40f64e3400b0f3b737dc6d4f64d05" - integrity sha512-sBGdETxC+/M4o/zKC0sl6sjWv62WFR/uzxrJ6uYyMLZOUlPnwzw0tKgVHOXxaAd5l2g8pEDM5RZ495GPQI77kg== +"@babel/types@^7.21.4": + version "7.21.4" + resolved "https://registry.yarnpkg.com/@babel/types/-/types-7.21.4.tgz#2d5d6bb7908699b3b416409ffd3b5daa25b030d4" + integrity sha512-rU2oY501qDxE8Pyo7i/Orqma4ziCOrby0/9mvbDUGEfvZjb279Nk9k19e2fiCxHbRRpY2ZyrgW1eq22mvmOIzA== dependencies: "@babel/helper-string-parser" "^7.19.4" "@babel/helper-validator-identifier" "^7.19.1"