From e54dc41ddd1e6e9aa46ae3a31e312e0b7d23fef8 Mon Sep 17 00:00:00 2001 From: Mateusz Titz Date: Tue, 27 Aug 2024 13:10:53 +0200 Subject: [PATCH 1/3] Improve react-compiler developer experience and docs --- .gitignore | 3 +++ contributingGuides/REACT_COMPILER.md | 34 ++++++++++++++++++++++++---- package.json | 1 + 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index f9d74fe950bd..3e899e3175ba 100644 --- a/.gitignore +++ b/.gitignore @@ -140,3 +140,6 @@ web-build/ # Jeykll docs/.bundle + +# Output of react compiler healthcheck dev script +react-compiler-output.txt diff --git a/contributingGuides/REACT_COMPILER.md b/contributingGuides/REACT_COMPILER.md index 144dd56100b7..ddd7aa0eaa1c 100644 --- a/contributingGuides/REACT_COMPILER.md +++ b/contributingGuides/REACT_COMPILER.md @@ -16,10 +16,11 @@ If the CI check fails for your PR, you need to fix the problem. If you're unsure ## How can I check what exactly prevents file from successful optimization or whether my fix for passing `react-compiler` actually works? -You can run `npm run react-compiler-healthcheck` and examine the output. This command will list the files that failed to compile and provide details on what caused the failures. The output can be extensive, so you may want to write it to a file for easier review: +You can run a dedicated script: `react-compiler-healthcheck-test` and examine the output. +This command will list the files that failed to compile with details on what caused the failures. It will then save this output to `./react-compiler-output.txt` file. Read and examine the output to find what specific error the react-compiler throws. ```bash -npm run react-compiler-healthcheck &> output.txt +npm run react-compiler-healthcheck-test ``` ## How to fix a particular problem? @@ -39,10 +40,13 @@ If you encounter this error, you need to add the `Ref` postfix to the variable n If you added a modification to `SharedValue`, you'll likely encounter this error. You can ignore this error for now because the current `react-native-reanimated` API is not compatible with `react-compiler` rules. Once [this PR](https://github.com/software-mansion/react-native-reanimated/pull/6312) is merged, we'll rewrite the code to be compatible with `react-compiler`. Until then, you can ignore this error. -### `manual memoization could not be preserved` +### `Existing manual memoization could not be preserved. [...]` +These types of errors usually occur when the calls to `useMemo` that were made manually are to complex for react-compiler to understand. React compiler is still experimental so unfortunately this can happen. -This error usually occurs when a dependency used inside a hook is omitted. This omission creates a memoization that is too complex to optimize automatically. Try including the missing dependencies. +One of the cases can be: +`The inferred dependencies did not match the manually specified dependencies` +This usually happens when a dependency used inside a hook is omitted. Try including the missing dependencies. Please be aware that `react-compiler` struggles with memoization of nested fields, i. e.: ```ts @@ -56,6 +60,28 @@ const selectedQboAccountName = useMemo(() => qboAccountOptions?.find(({id}) => i // which is great because it reduces the amount of the duplicated code ``` +Another case is: +`This value may be mutated later, which could cause the value to change unexpectedly` + +This usually happens when a value return from `useMemo` is later passed to some other function, and react-compiler doesn't know if the value will stay stable or be mutated. + +```ts +// ❌ such code triggers the error +const myResult = useMemo(() => SearchUtils.buildSearchQueryJSON(foobar), [foobar]); +// [...] some other code +const betterQuery = Utils.improveQuery(myResult); + +// ✅ this code can be compiled successfully +const {myResult, betterQuery} = useMemo(() => { + const result = SearchUtils.buildSearchQueryJSON(foobar); + + return { + myResult: result, + betterQuery: Utils.improveQuery(result) + } +},[foobar]); +``` + ### `Invalid nesting in program blocks or scopes` Such error may happen if we have a nested memoization, i. e.: diff --git a/package.json b/package.json index 2d5067bb8c08..df571fb230ab 100644 --- a/package.json +++ b/package.json @@ -62,6 +62,7 @@ "setup-https": "mkcert -install && mkcert -cert-file config/webpack/certificate.pem -key-file config/webpack/key.pem dev.new.expensify.com localhost 127.0.0.1", "e2e-test-runner-build": "node --max-old-space-size=8192 node_modules/.bin/ncc build tests/e2e/testRunner.ts -o tests/e2e/dist/", "react-compiler-healthcheck": "react-compiler-healthcheck --verbose", + "react-compiler-healthcheck-test": "react-compiler-healthcheck --verbose &> react-compiler-output.txt", "generate-search-parser": "peggy --format es -o src/libs/SearchParser/searchParser.js src/libs/SearchParser/searchParser.peggy ", "web:prod": "http-server ./dist --cors" }, From bb3c4c62c7db5bc75edd906d1e1a79e16b9eb5a6 Mon Sep 17 00:00:00 2001 From: Mateusz Titz Date: Tue, 27 Aug 2024 15:41:41 +0200 Subject: [PATCH 2/3] Improve readability of REACT_COMPILER.md --- contributingGuides/REACT_COMPILER.md | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/contributingGuides/REACT_COMPILER.md b/contributingGuides/REACT_COMPILER.md index ddd7aa0eaa1c..9414c72c1199 100644 --- a/contributingGuides/REACT_COMPILER.md +++ b/contributingGuides/REACT_COMPILER.md @@ -16,8 +16,7 @@ If the CI check fails for your PR, you need to fix the problem. If you're unsure ## How can I check what exactly prevents file from successful optimization or whether my fix for passing `react-compiler` actually works? -You can run a dedicated script: `react-compiler-healthcheck-test` and examine the output. -This command will list the files that failed to compile with details on what caused the failures. It will then save this output to `./react-compiler-output.txt` file. Read and examine the output to find what specific error the react-compiler throws. +You can run a dedicated script: `react-compiler-healthcheck-test` and examine the output. This command will list the files that failed to compile with details on what caused the failures. It will then save this output to `./react-compiler-output.txt` file. Read and examine the output to find what specific error the react-compiler throws. ```bash npm run react-compiler-healthcheck-test @@ -40,13 +39,15 @@ If you encounter this error, you need to add the `Ref` postfix to the variable n If you added a modification to `SharedValue`, you'll likely encounter this error. You can ignore this error for now because the current `react-native-reanimated` API is not compatible with `react-compiler` rules. Once [this PR](https://github.com/software-mansion/react-native-reanimated/pull/6312) is merged, we'll rewrite the code to be compatible with `react-compiler`. Until then, you can ignore this error. -### `Existing manual memoization could not be preserved. [...]` -These types of errors usually occur when the calls to `useMemo` that were made manually are to complex for react-compiler to understand. React compiler is still experimental so unfortunately this can happen. +### Existing manual memoization could not be preserved. [...] +These types of errors usually occur when the calls to `useMemo` that were made manually are too complex for react-compiler to understand. React compiler is still experimental so unfortunately this can happen. -One of the cases can be: -`The inferred dependencies did not match the manually specified dependencies` +Some specific cases of this error are described below. + +#### The inferred dependencies did not match the manually specified dependencies + +This usually happens when a dependency used inside a hook is omitted. Try including the missing dependencies. -This usually happens when a dependency used inside a hook is omitted. Try including the missing dependencies. Please be aware that `react-compiler` struggles with memoization of nested fields, i. e.: ```ts @@ -60,10 +61,9 @@ const selectedQboAccountName = useMemo(() => qboAccountOptions?.find(({id}) => i // which is great because it reduces the amount of the duplicated code ``` -Another case is: -`This value may be mutated later, which could cause the value to change unexpectedly` +#### This value may be mutated later, which could cause the value to change unexpectedly -This usually happens when a value return from `useMemo` is later passed to some other function, and react-compiler doesn't know if the value will stay stable or be mutated. +This usually happens when a value return from `useMemo` is later passed to some other function, and `react-compiler` doesn't know if the value will stay stable or be mutated. ```ts // ❌ such code triggers the error From 796a57123062f35077509e2d53b44ee6980c0b12 Mon Sep 17 00:00:00 2001 From: Mateusz Titz Date: Wed, 28 Aug 2024 14:47:53 +0200 Subject: [PATCH 3/3] fix doc --- contributingGuides/REACT_COMPILER.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contributingGuides/REACT_COMPILER.md b/contributingGuides/REACT_COMPILER.md index 9414c72c1199..520cbd7b164a 100644 --- a/contributingGuides/REACT_COMPILER.md +++ b/contributingGuides/REACT_COMPILER.md @@ -63,7 +63,7 @@ const selectedQboAccountName = useMemo(() => qboAccountOptions?.find(({id}) => i #### This value may be mutated later, which could cause the value to change unexpectedly -This usually happens when a value return from `useMemo` is later passed to some other function, and `react-compiler` doesn't know if the value will stay stable or be mutated. +This usually happens when the value returned from `useMemo` is later passed to some other function, and `react-compiler` doesn't know if the value will stay stable or be mutated. ```ts // ❌ such code triggers the error