-
Notifications
You must be signed in to change notification settings - Fork 49.6k
[compiler] [playground] Show internals toggle #34399
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
Conversation
Open to feedback! Right now the state of the toggle switch is in the store (and associated with the URL), should I change it to be associated only with a user's localStorage? |
In my opinion, broadly speaking there's 2 categories of users of the playground: 1. Compiler devs, 2. React devs. The former will want to see all the tabs, while the latter may want to see the passes if they're curious but mainly need to look at the output tab and the config panel. So this toggle makes sense to me, but I would maybe default it to showing internals, and when it's off only show the config panel and output tab. The reason it's useful to show the config panel is because users may have passed some options to the compiler and to provide us a repro we would want their same config. Bonus points if we can make an curious what @josephsavona and @mofeiZ think! |
If we save the setting to local storage, and turn internals off by default, then the compiler devs would be able to enable it once and always have it on which would be nice too |
For the config it would be nice to have a form field editor like other playgrounds: https://babeljs.io/repl |
The form field editor is a good suggestion! Curious what the guys that work on the compiler a lot think about this; I think it could be worth the dev effort if it'll have good use |
} | ||
} | ||
|
||
const filteredTabs = useMemo(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this memoization? The compiler is enabled for this app
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, doing these here means that tabify
is still doing the work to render out <TextTabContent/>
components for each tab, on every source change. Then we're just ignoring most of them in the case of the filtered view.
Instead, we could skip the tabify work altogether. Maybe something like
for (const [passName, results] of compilerOutput.results) {
if (store.showInternals === false && (passName !== 'Output' || passName !== 'SourceMap' )) {
continue;
}
// ...
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
*/ | ||
closeSnackbar(); | ||
dispatchStore({type: 'setStore', payload: {store: defaultStore}}); | ||
dispatchStore({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Super nit: we don't need to preserve showInternals
when resetting the playground. I think this feature as more of a "force reset" which should entirely clear client state
export const defaultStore: Store = { | ||
source: index, | ||
config: defaultConfig, | ||
showInternals: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me! compiler developers can manually toggle on show internals
when using playground for the first time. All later sessions would be able to load this setting from localStorage
*/ | ||
export function saveStore(store: Store): void { | ||
const hash = encodeStore(store); | ||
localStorage.setItem( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm I'm curious -- why does encodeStore(...)
not work for also saving showInternals
?
const encodedSource = encodedSourceFromUrl || encodedSourceFromLocal; | ||
|
||
// Prioritize local storage for showInternals field | ||
const showInternals = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see, is the reasoning that if someone links a playground example (with showInternals = false
), we want the localStorage to override this?
Let's keep things simple for now -- I don't mind toggling showInternals
back when I load a URL.
The reasoning for this is that we might eventually use showInternals
in playground config (e.g. not logging passes unless show internals is enabled). Having URL loading be straightforward is one less thing to think about when debugging
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok, I'll revert this then!
2352b37
to
89b35c5
Compare
<!-- 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 Added a "Show Internals" toggle switch to either show only the Config, Input, Output, and Source Map tabs, or these tabs + all the additional compiler options. The open/close state of these tabs will be preserved (unless on page refresh, which is the same as the currently functionality). <!-- Explain the **motivation** for making this change. What existing problem does the pull request solve? --> ## How did you test this change? <!-- 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. --> https://github.com/user-attachments/assets/8eb0f69e-360c-4e9b-9155-7aa185a0c018 DiffTrain build for [d4374b3](facebook@d4374b3)
<!-- 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 Added a "Show Internals" toggle switch to either show only the Config, Input, Output, and Source Map tabs, or these tabs + all the additional compiler options. The open/close state of these tabs will be preserved (unless on page refresh, which is the same as the currently functionality). <!-- Explain the **motivation** for making this change. What existing problem does the pull request solve? --> ## How did you test this change? <!-- 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. --> https://github.com/user-attachments/assets/8eb0f69e-360c-4e9b-9155-7aa185a0c018 DiffTrain build for [d4374b3](facebook@d4374b3)
Summary
Added a "Show Internals" toggle switch to either show only the Config, Input, Output, and Source Map tabs, or these tabs + all the additional compiler options. The open/close state of these tabs will be preserved (unless on page refresh, which is the same as the currently functionality).
How did you test this change?
Screen.Recording.2025-09-05.at.11.30.37.AM.mov