[playground] bug fixes & UX improvements #34499
Conversation
| config: value, | ||
| }, | ||
| }); | ||
| }, 500); // 500ms debounce delay |
There was a problem hiding this comment.
Can we use useDeferredValue() instead of manually debouncing?
There was a problem hiding this comment.
useDeferredValue was already being used previously to update the output: https://github.com/facebook/react/blob/main/compiler/apps/playground/components/Editor/EditorImpl.tsx#L329.
It seemed like these updates were still happening too quickly while typing in the config editor so I tried adding a manual debouncing mechanism with a longer timeout window.
There was a problem hiding this comment.
@josephsavona we're adding debouncing to reduce thrash in the output panel from JS parse errors
EugeneChoi4
changed the title
| <StoreContext.Provider value={store}> | ||
| <StoreDispatchContext.Provider value={dispatch}> | ||
| {children} | ||
| {isPageReady ? children : null} |
There was a problem hiding this comment.
Ah gotcha, we're now waiting until Store is initialized before revealing the page
There was a problem hiding this comment.
Yup! This seemed like the best way to init with url/local storage
| const dispatchStore = useStoreDispatch(); | ||
| const debounceTimerRef = useRef<NodeJS.Timeout | null>(null); | ||
|
|
||
| // Cleanup timeout on unmount |
There was a problem hiding this comment.
Do you need this effect for cleanup?
In the unlikely scenario that you finished typing and closed the config editor within 500ms, you'd still watch the last input change to dispatch.
There was a problem hiding this comment.
Just for my understanding, when would it be appropriate to have the cleanup effect? If the ExpandedEditor component had some state that was being manipulated by the timer, would it then be necessary?
There was a problem hiding this comment.
If for example this used a setInterval or otherwise initialized a resource that could cause memory leaks or other side effects by staying around then you want to clean that up. In this case, the timer reference is going to expire at the end of the timeout anyway, so you'd just want to decide if its important if that function is canceled after an unmount or not.
| }); | ||
| const text = | ||
| (await page.locator('.monaco-editor').nth(1).allInnerTexts()) ?? []; | ||
| (await page.locator('.monaco-editor').nth(3).allInnerTexts()) ?? []; |
There was a problem hiding this comment.
Seems like this has gone back and forth a couple times. As a follow up there could be a test that starts the app both with internals showing and not showing.
Also/alternatively adding some testID to the tabs would allow for better selecting. like '[data-testid="config-tab"] .monaco-editor'
There was a problem hiding this comment.
Yup, I'm actually cleaning up/adding more tests in another PR right now so this will be addressed there.
| @@ -1,56 +0,0 @@ | |||
| /** | |||
| * Copyright (c) Meta Platforms, Inc. and affiliates. | |||
There was a problem hiding this comment.
Gotcha, we're now using app/page.tsx. Makes sense to delete this
[diff facebook/react@84af9085...d415fd3e](facebook/react@84af908...d415fd3) <details> <summary>React upstream changes</summary> - facebook/react#34536 - facebook/react#34534 - facebook/react#34531 - facebook/react#34499 - facebook/react#34523 - facebook/react#34511 - facebook/react#34516 - facebook/react#34455 - facebook/react#34488 - facebook/react#34521 </details>
<!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please provide enough information so that others can review your pull request. The three fields below are mandatory. Before submitting a pull request, please make sure the following is done: 1. Fork [the repository](https://github.com/facebook/react) and create your branch from `main`. 2. Run `yarn` in the repository root. 3. If you've fixed a bug or added code that should be tested, add tests! 4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch TestName` is helpful in development. 5. Run `yarn test --prod` to test in the production environment. It supports the same options as `yarn test`. 6. If you need a debugger, run `yarn test --debug --watch TestName`, open `chrome://inspect`, and press "Inspect". 7. Format your code with [prettier](https://github.com/prettier/prettier) (`yarn prettier`). 8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only check changed files. 9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`). 10. If you haven't already, complete the CLA. Learn more about contributing: https://reactjs.org/docs/how-to-contribute.html --> ## Summary Made many small changes to the compiler playground to improve user experience. Removed any "Loading" indicators that would flash in before a component would finish loading in. Additionally, before users would see the "Show Internals" button toggling from false to true if they had set it at true previously. I was able to refactor the URL/local storage loading so that the `Store` would be fully initialized before the components would load in. Attempted to integrate `<Activity>` into showing/hiding these different editors, but the current state of [monaco editors](https://github.com/suren-atoyan/monaco-react) does not allow for this. I created an issue for them to address: suren-atoyan/monaco-react#753 Added a debounce to the config editor so every key type wouldn't cause the output panel to respond instantly. Users can type for 500 ms before an error is thrown at them. <!-- Explain the **motivation** for making this change. What existing problem does the pull request solve? --> ## How did you test this change? Here is what loading the page would look like before (not sure why its so blurry): https://github.com/user-attachments/assets/58f4281a-cc02-4141-b9b5-f70d6ace12a2 Here is how it looks now: https://github.com/user-attachments/assets/40535165-fc7c-44fb-9282-9c7fa76e7d53 Here is the debouncing: https://github.com/user-attachments/assets/e4ab29e4-1afd-4249-beca-671fb6542f5e <!-- Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes the user interface. How exactly did you verify that your PR solves the issue you wanted to solve? If you leave this empty, your PR will very likely be closed. --> DiffTrain build for [6eda534](facebook@6eda534)
<!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please provide enough information so that others can review your pull request. The three fields below are mandatory. Before submitting a pull request, please make sure the following is done: 1. Fork [the repository](https://github.com/facebook/react) and create your branch from `main`. 2. Run `yarn` in the repository root. 3. If you've fixed a bug or added code that should be tested, add tests! 4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch TestName` is helpful in development. 5. Run `yarn test --prod` to test in the production environment. It supports the same options as `yarn test`. 6. If you need a debugger, run `yarn test --debug --watch TestName`, open `chrome://inspect`, and press "Inspect". 7. Format your code with [prettier](https://github.com/prettier/prettier) (`yarn prettier`). 8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only check changed files. 9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`). 10. If you haven't already, complete the CLA. Learn more about contributing: https://reactjs.org/docs/how-to-contribute.html --> ## Summary Made many small changes to the compiler playground to improve user experience. Removed any "Loading" indicators that would flash in before a component would finish loading in. Additionally, before users would see the "Show Internals" button toggling from false to true if they had set it at true previously. I was able to refactor the URL/local storage loading so that the `Store` would be fully initialized before the components would load in. Attempted to integrate `<Activity>` into showing/hiding these different editors, but the current state of [monaco editors](https://github.com/suren-atoyan/monaco-react) does not allow for this. I created an issue for them to address: suren-atoyan/monaco-react#753 Added a debounce to the config editor so every key type wouldn't cause the output panel to respond instantly. Users can type for 500 ms before an error is thrown at them. <!-- Explain the **motivation** for making this change. What existing problem does the pull request solve? --> ## How did you test this change? Here is what loading the page would look like before (not sure why its so blurry): https://github.com/user-attachments/assets/58f4281a-cc02-4141-b9b5-f70d6ace12a2 Here is how it looks now: https://github.com/user-attachments/assets/40535165-fc7c-44fb-9282-9c7fa76e7d53 Here is the debouncing: https://github.com/user-attachments/assets/e4ab29e4-1afd-4249-beca-671fb6542f5e <!-- Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes the user interface. How exactly did you verify that your PR solves the issue you wanted to solve? If you leave this empty, your PR will very likely be closed. --> DiffTrain build for [6eda534](facebook@6eda534)
4 participants
Summary
Made many small changes to the compiler playground to improve user experience. Removed any "Loading" indicators that would flash in before a component would finish loading in. Additionally, before users would see the "Show Internals" button toggling from false to true if they had set it at true previously. I was able to refactor the URL/local storage loading so that the
Storewould be fully initialized before the components would load in.Attempted to integrate
<Activity>into showing/hiding these different editors, but the current state of monaco editors does not allow for this. I created an issue for them to address: suren-atoyan/monaco-react#753Added a debounce to the config editor so every key type wouldn't cause the output panel to respond instantly. Users can type for 500 ms before an error is thrown at them.
How did you test this change?
Here is what loading the page would look like before (not sure why its so blurry):
Screen.Recording.2025-09-17.at.7.46.18.PM.mov
Here is how it looks now:
Screen.Recording.2025-09-17.at.7.48.07.PM.mov
Here is the debouncing:
Screen.Recording.2025-09-17.at.7.59.48.PM.mov