feat(playground): add JSON editor for Playground settings#4116
feat(playground): add JSON editor for Playground settings#4116mkosei wants to merge 6 commits intobiomejs:mainfrom
Conversation
✅ Deploy Preview for biomejs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughAdds JSON-based editing and export of the Playground's effective Biome configuration. Introduces 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/playground/buildBiomeConfiguration.ts`:
- Around line 209-224: The code currently seeds parseBiomeConfiguration() with
defaultPlaygroundState.settings which silently resets playground-only properties
like analyzerFixMode and gritTargetLanguage when the user clicks Apply; change
the logic to start from the existing current settings (or merge the parsed
subset on top of the existing settings) instead of using
defaultPlaygroundState.settings so playground-specific fields are
preserved—update the initialization that assigns defaults (the variable named
defaults) and the return object construction in
parseBiomeConfiguration()/buildBiomeConfiguration to merge parsed values into
the current settings rather than replacing them.
- Around line 43-69: getLintRulesConfiguration currently only handles the two
enum presets (LINT_RULES.recommended and LINT_RULES.all) and collapses any other
value into { recommended: false }, which loses custom selections; change
getLintRulesConfiguration to detect when lintRules is already a LinterRules
object (i.e., a custom selection) and return it unchanged, while keeping the
existing branches for the enum presets, and apply the same fix to the
corresponding reverse-mapper referenced around lines 71-103 so custom lint rule
objects round-trip intact (use the function name getLintRulesConfiguration and
the other mapper's name to locate and update the logic).
In `@src/playground/components/SettingsJsonEditorModal.tsx`:
- Around line 38-46: The effect currently re-initialises the editor whenever
settings changes, wiping in-progress edits; update the initialization so it only
runs on the closed→open transition: track the previous isOpen (e.g. with a
useRef or usePrevious helper) and inside the useEffect that watches isOpen only
call setJsonValue(JSON.stringify(createEditableConfiguration(settings), null,
2)), setJsonError(null) and setCopyStatus("idle") when isOpen is true and
previousIsOpen was false; also remove settings from the effect dependency array
so edits aren't reset while the modal is open (keep references to
createEditableConfiguration, setJsonValue, setJsonError, setCopyStatus as
needed).
In `@src/styles/playground/_settings.css`:
- Around line 301-323: Add a light foreground color to the modal container so
headings and error text inherit a readable color: update
.settings-json-modal__content to set a light text color (use the theme token if
available, e.g. a primary/light text token) so children like
.settings-json-modal__header, .settings-json-modal__actions and
.settings-json-modal__subheader inherit it; if you prefer explicitness, also add
the same color rule to .settings-json-modal__header,
.settings-json-modal__actions and .settings-json-modal__subheader to ensure
consistent contrast across themes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3fe09008-4ea9-4650-96b5-090a1aacb09d
📒 Files selected for processing (4)
src/playground/buildBiomeConfiguration.tssrc/playground/components/SettingsJsonEditorModal.tsxsrc/playground/tabs/SettingsTab.tsxsrc/styles/playground/_settings.css
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/playground/buildBiomeConfiguration.ts (1)
252-252:⚠️ Potential issue | 🟠 MajorApplying JSON still resets playground-only settings.
Because
defaultsis hard-wired todefaultPlaygroundState.settings,analyzerFixModeandgritTargetLanguagesnap back tosafeFixesandJavaScripton every Apply. Please merge the parsed subset over the current settings instead of rebuilding from global defaults.Also applies to: 267-340
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/playground/buildBiomeConfiguration.ts` at line 252, The code currently binds defaults to defaultPlaygroundState.settings (const defaults = defaultPlaygroundState.settings) which causes playground-only keys like analyzerFixMode and gritTargetLanguage to be reset on Apply; instead, merge the parsed settings subset over the current playground settings (not the global defaults). Locate the block(s) using defaults in buildBiomeConfiguration.ts (and the similar block around 267-340) and replace the rebuild-from-defaults logic with a shallow merge such as: take the existing currentSettings object (the live playground state/settings variable) and spread parsedSettings over it (currentSettings = { ...currentSettings, ...parsedSettings }) so only provided keys are updated while analyzerFixMode, gritTargetLanguage and other local-only fields are preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/playground/buildBiomeConfiguration.ts`:
- Around line 70-80: The LINT_RULES.all branch currently omits the source lint
group so "all" is not truly all; update the serializer in
buildBiomeConfiguration (the case for LINT_RULES.all) to include source: "on"
alongside a11y, nursery, complexity, etc., and likewise add source: "on" to the
parallel block around lines 105-133; also update the reverse-mapping logic that
maps a serialized config back to LINT_RULES.all to check the source group as
well before returning LINT_RULES.all so the round-trip correctly recognizes the
source group.
---
Duplicate comments:
In `@src/playground/buildBiomeConfiguration.ts`:
- Line 252: The code currently binds defaults to defaultPlaygroundState.settings
(const defaults = defaultPlaygroundState.settings) which causes playground-only
keys like analyzerFixMode and gritTargetLanguage to be reset on Apply; instead,
merge the parsed settings subset over the current playground settings (not the
global defaults). Locate the block(s) using defaults in
buildBiomeConfiguration.ts (and the similar block around 267-340) and replace
the rebuild-from-defaults logic with a shallow merge such as: take the existing
currentSettings object (the live playground state/settings variable) and spread
parsedSettings over it (currentSettings = { ...currentSettings,
...parsedSettings }) so only provided keys are updated while analyzerFixMode,
gritTargetLanguage and other local-only fields are preserved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 843c1228-cf18-4264-8289-9856f5c560a7
📒 Files selected for processing (1)
src/playground/buildBiomeConfiguration.ts
|
I’d like to get the maintainers’ opinion on one detail. Right now the JSON editor includes a $schema entry. It doesn’t affect how the Playground itself behaves, but I wanted to ask whether you think including it here makes sense. |
ematipico
left a comment
ematipico
left a comment
There was a problem hiding this comment.
It would be nice to make the modal full screen on mobile
Also, after the refactor, we are using so many repeated strings. We should re-use the ones we already have
| }: SettingsJsonEditorModalProps) { | ||
| const dialogRef = useRef<HTMLDialogElement>(null); | ||
| const wasOpenRef = useRef(false); | ||
| const [jsonValue, setJsonValue] = useState(""); |
There was a problem hiding this comment.
An empty JSON string is invalid. Use an empty object instead.
Also, don't call it "json value", maybe configurationAsJson?
src/playground/tabs/SettingsTab.tsx
Outdated
| <> | ||
| <section className="settings-json-actions"> | ||
| <button type="button" onClick={() => setIsJsonModalOpen(true)}> | ||
| Edit as JSON |
| }) as Array<[LintRuleGroup, Record<string, string>]>; | ||
|
|
||
| function getBiomeIndentStyle(indentStyle: PlaygroundSettings["indentStyle"]) { | ||
| return indentStyle === IndentStyle.Tab ? "tab" : "space"; |
There was a problem hiding this comment.
We should re-use the strings we have in IndentStyle. Same for the rest of enums
| case Expand.Always: | ||
| return "always"; | ||
| case Expand.Never: | ||
| return "never"; | ||
| default: | ||
| return "auto"; |
There was a problem hiding this comment.
Same, let's re use the strings, now repeating
mkosei
force-pushed
the
feature/edit-playground-config-as-json
branch
from
c9b1bbf to
dbe6941
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/playground/buildBiomeConfiguration.ts (1)
167-177:⚠️ Potential issue | 🟠 Major
allstill is not quite all.This path still hard-codes the lint groups and skips
source, so exportingLINT_RULES.alldrops one ruleset and the reverse mapper will not recognise it on import. Please derive both branches fromLINT_RULE_GROUPShere, rather than keeping a second “all, except that one” list.Also applies to: 203-229
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/playground/buildBiomeConfiguration.ts` around lines 167 - 177, The LINT_RULES.all branch currently hard-codes groups and omits the "source" group; replace that hard-coded object by deriving its entries from LINT_RULE_GROUPS (e.g., map/reduce over LINT_RULE_GROUPS to produce an object with each group -> "on") so the exported "all" truly includes every group (including "source") and the reverse mapper will recognise it; make the same change for the other identical block flagged (the duplicate at the other occurrence) so both branches are generated from LINT_RULE_GROUPS rather than separate lists.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/playground/buildBiomeConfiguration.ts`:
- Around line 27-30: Reverse mapper helpers (e.g., getPlaygroundIndentStyle,
getPlaygroundSemicolons, getPlaygroundLintRules, getPlaygroundAttributePosition
and the other helper functions in the same file) must not fall back to
defaultPlaygroundState; instead they should return an explicit “not specified”
value (undefined/null per the function signature) when the parsed JSON omits a
key. Change each helper to stop returning defaultPlaygroundState.* and return
undefined for missing/unspecified inputs, and then ensure
parseBiomeConfiguration() performs the actual coalescing with defaults.* and
currentSettings so omitted fields don’t reset the playground and "auto" values
can override existing settings.
In `@src/playground/components/SettingsJsonEditorModal.tsx`:
- Around line 88-97: The applyJsonSettings function currently trusts JSON.parse
output; instead validate the parsed object (parsed from configurationAsJson) for
the expected shape of Configuration/PlaygroundSettings before calling
onApply(parseBiomeConfiguration(parsed, settings)); perform a runtime schema
check (e.g., required keys and types or use a lightweight validator) on parsed
and on the result of parseBiomeConfiguration if needed, and if validation fails
call setJsonError with a clear message and do not call onApply or onClose; keep
parseBiomeConfiguration, onApply, onClose and setJsonError usage but add these
validation gates to reject non-conforming configs like wrong types for
formatter.lineWidth.
---
Duplicate comments:
In `@src/playground/buildBiomeConfiguration.ts`:
- Around line 167-177: The LINT_RULES.all branch currently hard-codes groups and
omits the "source" group; replace that hard-coded object by deriving its entries
from LINT_RULE_GROUPS (e.g., map/reduce over LINT_RULE_GROUPS to produce an
object with each group -> "on") so the exported "all" truly includes every group
(including "source") and the reverse mapper will recognise it; make the same
change for the other identical block flagged (the duplicate at the other
occurrence) so both branches are generated from LINT_RULE_GROUPS rather than
separate lists.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 875b1e92-9cd4-4ecb-b125-ed2384ce611e
📒 Files selected for processing (4)
src/playground/buildBiomeConfiguration.tssrc/playground/components/SettingsJsonEditorModal.tsxsrc/playground/tabs/SettingsTab.tsxsrc/styles/playground/_settings.css
🚧 Files skipped from review as they are similar to previous changes (2)
- src/playground/tabs/SettingsTab.tsx
- src/styles/playground/_settings.css
| function getPlaygroundIndentStyle(indentStyle: IndentStyle | undefined) { | ||
| return indentStyle === IndentStyle.Space | ||
| ? IndentStyle.Space | ||
| : defaultPlaygroundState.settings.indentStyle; |
There was a problem hiding this comment.
Do not fall back to defaultPlaygroundState inside the reverse mappers.
These helpers run whilst merging parsed JSON over currentSettings, but they return global defaults for missing keys. That means omitting semicolons, indentStyle, lintRules, etc. from the JSON can still reset the playground, and attributePosition: "auto" cannot override a current Multiline value. Let the helpers return “not specified” and coalesce with defaults.* in parseBiomeConfiguration() instead.
Also applies to: 41-46, 60-68, 95-100, 107-112, 121-126, 137-142, 197-242, 360-418
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/playground/buildBiomeConfiguration.ts` around lines 27 - 30, Reverse
mapper helpers (e.g., getPlaygroundIndentStyle, getPlaygroundSemicolons,
getPlaygroundLintRules, getPlaygroundAttributePosition and the other helper
functions in the same file) must not fall back to defaultPlaygroundState;
instead they should return an explicit “not specified” value (undefined/null per
the function signature) when the parsed JSON omits a key. Change each helper to
stop returning defaultPlaygroundState.* and return undefined for
missing/unspecified inputs, and then ensure parseBiomeConfiguration() performs
the actual coalescing with defaults.* and currentSettings so omitted fields
don’t reset the playground and "auto" values can override existing settings.
| function applyJsonSettings() { | ||
| try { | ||
| const parsed = JSON.parse(configurationAsJson) as Configuration; | ||
| onApply(parseBiomeConfiguration(parsed, settings)); | ||
| onClose(); | ||
| } catch (error) { | ||
| setJsonError( | ||
| error instanceof Error ? error.message : "Invalid JSON settings.", | ||
| ); | ||
| } |
There was a problem hiding this comment.
Validate the parsed object before applying it.
JSON.parse only proves the text is valid JSON. A payload like { "formatter": { "lineWidth": "wide" } } still sails through here and can write invalid runtime types into PlaygroundSettings, which is a tidy way to make the playground sad. Please reject non-conforming configs before calling onApply.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/playground/components/SettingsJsonEditorModal.tsx` around lines 88 - 97,
The applyJsonSettings function currently trusts JSON.parse output; instead
validate the parsed object (parsed from configurationAsJson) for the expected
shape of Configuration/PlaygroundSettings before calling
onApply(parseBiomeConfiguration(parsed, settings)); perform a runtime schema
check (e.g., required keys and types or use a lightweight validator) on parsed
and on the result of parseBiomeConfiguration if needed, and if validation fails
call setJsonError with a clear message and do not call onApply or onClose; keep
parseBiomeConfiguration, onApply, onClose and setJsonError usage but add these
validation gates to reject non-conforming configs like wrong types for
formatter.lineWidth.
2 participants
Summary
Closes #4113
Edit as JSONentry point in the Playground settings tabdialog-based JSON editor for editing Playground settingsbiome.json-like configuration that can also be copiedNotes
$schemavalue$schemaversion manually if needed