-
Notifications
You must be signed in to change notification settings - Fork 3
17.0.2 memory patch #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 17.0.2
Are you sure you want to change the base?
Conversation
Hi, you wrote a comment that these changes are a HACK, what do you think we need to do to fix this memory leak by a good way that gonna be merged to facebook/react repo ? |
…paces (facebook#33409) ## Summary Problem #1: Running the `link-compiler.sh` bash script via `"prebuild"` script fails if a developer has cloned the `react` repo into a folder that contains _any_ spaces. 3 tests fail because of this. <img width="1003" alt="fail-1" src="https://github.com/user-attachments/assets/1fbfa9ce-4f84-48d7-b49c-b6e967b8c7ca" /> <img width="1011" alt="fail-2" src="https://github.com/user-attachments/assets/0a8c6371-a2df-4276-af98-38f4784cf0da" /> <img width="1027" alt="fail-3" src="https://github.com/user-attachments/assets/1c4f4429-800c-4b44-b3da-a59ac85a16b9" /> For example, my current folder is: `/Users/wes/Development/Open Source Contributions/react` The link compiler error returns: `./scripts/react-compiler/link-compiler.sh: line 15: cd: /Users/wes/Development/Open: No such file or directory` Problem #2: 1 test in `ReactChildren-test.js` fails due the existing stack trace regex which should be lightly revised. `([^(\[\n]+)[^\n]*/g` is more robust for stack traces: it captures the function/class name (with dots) and does not break on spaces in file paths. `([\S]+)[^\n]*/g` is simpler but breaks if there are spaces and doesn't handle dotted names well. Additionally, we trim the whitespace off the name to resolve extra spaces breaking this test as well: ``` - in div (at **) + in div (at **) ``` <img width="987" alt="fail-4" src="https://github.com/user-attachments/assets/56a673bc-513f-4458-95b2-224129c77144" /> All of the above tests pass if I hyphenate my local folder: `/Users/wes/Development/Open-Source-Contributions/react` I selfishly want to keep spaces in my folder names. 🫣 ## How did you test this change? **npx yarn prebuild** Before: <img width="896" alt="Screenshot at Jun 01 11-42-56" src="https://github.com/user-attachments/assets/4692775c-1e5c-4851-9bd7-e12ed5455e47" /> After: <img width="420" alt="Screenshot at Jun 01 11-43-42" src="https://github.com/user-attachments/assets/4e303c00-02b7-4540-ba19-927b2d7034fb" /> **npx yarn test** **npx yarn test ./packages/react/src/\_\_tests\_\_/ReactChildren-test.js** **npx yarn test -r=xplat --env=development --variant=true --ci --shard=3/5** Before: <img width="438" alt="before" src="https://github.com/user-attachments/assets/f5eedb22-18c3-4124-a04b-daa95c0f7652" /> After: <img width="439" alt="after" src="https://github.com/user-attachments/assets/a94218ba-7c6a-4f08-85d3-57540e9d0029" /> <img width="650" alt="Screenshot at Jun 02 18-03-39" src="https://github.com/user-attachments/assets/3eae993c-a56b-46c8-ae02-d249cb053fe7" /> <img width="685" alt="Screenshot at Jun 03 12-53-47" src="https://github.com/user-attachments/assets/5b2caa33-d3dc-4804-981d-52cb10b6226f" />
This pull request has been automatically marked as stale. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated. |
Changes from discord#1 applied to https://github.com/facebook/react/tree/17.0.2. Equivalent changes are included in React 18 so these patches will not be needed.
discord@b0194a0 has been merged upstream in facebook#16115
react-dom
package has been built on branch https://github.com/paulshen/react/tree/17.0.2-memory-patch-buildUsage instructions
Update
package.json
with following