-
Notifications
You must be signed in to change notification settings - Fork 49.8k
[compiler][playground] (1/N) Config override panel #34303
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
Changes from all commits
8dcf894
495220d
d00e899
6e66d10
1fe6b36
2838933
2bf6324
3320a66
135dd0a
975da76
0c89de7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| /** | ||
| * Copyright (c) Meta Platforms, Inc. and affiliates. | ||
| * | ||
| * This source code is licensed under the MIT license found in the | ||
| * LICENSE file in the root directory of this source tree. | ||
| */ | ||
|
|
||
| import MonacoEditor, {loader, type Monaco} from '@monaco-editor/react'; | ||
| import {parseConfigPragmaAsString} from 'babel-plugin-react-compiler'; | ||
| import type {editor} from 'monaco-editor'; | ||
| import * as monaco from 'monaco-editor'; | ||
| import parserBabel from 'prettier/plugins/babel'; | ||
| import * as prettierPluginEstree from 'prettier/plugins/estree'; | ||
| import * as prettier from 'prettier/standalone'; | ||
| import {useState, useEffect} from 'react'; | ||
| import {Resizable} from 're-resizable'; | ||
| import {useStore} from '../StoreContext'; | ||
| import {monacoOptions} from './monacoOptions'; | ||
|
|
||
| loader.config({monaco}); | ||
|
|
||
| export default function ConfigEditor(): JSX.Element { | ||
| const [, setMonaco] = useState<Monaco | null>(null); | ||
| const store = useStore(); | ||
|
|
||
| // Parse string-based override config from pragma comment and format it | ||
| const [configJavaScript, setConfigJavaScript] = useState(''); | ||
|
|
||
| useEffect(() => { | ||
| const pragma = store.source.substring(0, store.source.indexOf('\n')); | ||
| const configString = `(${parseConfigPragmaAsString(pragma)})`; | ||
|
|
||
| prettier | ||
| .format(configString, { | ||
| semi: true, | ||
| parser: 'babel-ts', | ||
| plugins: [parserBabel, prettierPluginEstree], | ||
| }) | ||
| .then(formatted => { | ||
| setConfigJavaScript(formatted); | ||
| }) | ||
| .catch(error => { | ||
| console.error('Error formatting config:', error); | ||
| setConfigJavaScript('({})'); // Return empty object if not valid for now | ||
| //TODO: Add validation and error handling for config | ||
| }); | ||
| console.log('Config:', configString); | ||
| }, [store.source]); | ||
|
|
||
| const handleChange: (value: string | undefined) => void = value => { | ||
| if (!value) return; | ||
|
|
||
| // TODO: Implement sync logic to update pragma comments in the source | ||
| console.log('Config changed:', value); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's disable the editor while we don't have this hooked up. Output tab does this for example. In a future PR when the handler is added it can be re-enabled. |
||
| }; | ||
|
|
||
| const handleMount: ( | ||
| _: editor.IStandaloneCodeEditor, | ||
| monaco: Monaco, | ||
| ) => void = (_, monaco) => { | ||
| setMonaco(monaco); | ||
|
|
||
| const uri = monaco.Uri.parse(`file:///config.js`); | ||
| const model = monaco.editor.getModel(uri); | ||
| if (model) { | ||
| model.updateOptions({tabSize: 2}); | ||
| } | ||
| }; | ||
|
|
||
| return ( | ||
| <div className="relative flex flex-col flex-none border-r border-gray-200"> | ||
| <h2 className="p-4 duration-150 ease-in border-b cursor-default border-grey-200 font-light text-secondary"> | ||
| Config Overrides | ||
| </h2> | ||
| <Resizable | ||
| minWidth={300} | ||
| maxWidth={600} | ||
| defaultSize={{width: 350, height: 'auto'}} | ||
| enable={{right: true}} | ||
| className="!h-[calc(100vh_-_3.5rem_-_4rem)]"> | ||
| <MonacoEditor | ||
| path={'config.js'} | ||
| language={'javascript'} | ||
| value={configJavaScript} | ||
| onMount={handleMount} | ||
| onChange={handleChange} | ||
| options={{ | ||
| ...monacoOptions, | ||
| readOnly: true, | ||
| lineNumbers: 'off', | ||
| folding: false, | ||
| renderLineHighlight: 'none', | ||
| scrollBeyondLastLine: false, | ||
| hideCursorInOverviewRuler: true, | ||
| overviewRulerBorder: false, | ||
| overviewRulerLanes: 0, | ||
| fontSize: 12, | ||
| }} | ||
| /> | ||
| </Resizable> | ||
| </div> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,7 +48,10 @@ export { | |
| printReactiveFunction, | ||
| printReactiveFunctionWithOutlined, | ||
| } from './ReactiveScopes'; | ||
| export {parseConfigPragmaForTests} from './Utils/TestUtils'; | ||
| export { | ||
| parseConfigPragmaForTests, | ||
| parseConfigPragmaAsString, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure we want this to be a top-level export of the compiler package. Especially if this logic is specific to the playground app. Though I see Seems like how you had it before in the ConfigEditor component module would be ok.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The compiler package exposes a lot of implementation details already in its exports so either is fine 😅 It isn't necessary to expose this as a top level export though, you can import directly from a compiler source file. This is because next-js (e.g. compiler playground) does its own bundling which means it currently recursively traverses |
||
| } from './Utils/TestUtils'; | ||
| declare global { | ||
| let __DEV__: boolean | null | undefined; | ||
| } | ||
|
|
||
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.
You could compare this against the previous value to bail out of the effect early if you don't need to sync again.
An alternative approach (can be a followup PR) is to avoid this effect altogether by moving this into the event. Currently the flow is
If we refactor the store to have a config property, it could be
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.
Thank you for the suggestions! Yea definitely planning to fix the re-renders in a later commit, also going to try to fix the rerenders of the other components that use useEffect and useMemo incorrectly.