Controls: Force object control JSON mode to reset #33330
Conversation
|
View your CI Pipeline Execution ↗ for commit 023ee26
☁️ Nx Cloud last updated this comment at |
📝 WalkthroughWalkthroughAdds a memoized Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5–10 minutes ✨ Finishing touches
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: 0
🧹 Nitpick comments (1)
code/addons/docs/src/blocks/controls/Object.tsx (1)
210-213: Consider usingnullinstead of''as the fallback value.The fallback to
''is semantically imprecise:JSON.stringify('')produces'""'rather than a representation of absence. While this computed value is never used in practice (due to the early return at line 215 whenhasDatais false), usingnullas the fallback would be more semantically correct.Apply this diff for better semantic clarity:
- return JSON.stringify(data ?? '', null, 2); + return JSON.stringify(data ?? null, null, 2);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/addons/docs/src/blocks/controls/Object.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESLint and Prettier configurations that are enforced in the codebase
Files:
code/addons/docs/src/blocks/controls/Object.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode
Files:
code/addons/docs/src/blocks/controls/Object.tsx
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{ts,tsx,js,jsx,mjs}: Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Use client-side logger from 'storybook/internal/client-logger' for browser code
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/addons/docs/src/blocks/controls/Object.tsx
code/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions that need to be tested from their modules
Files:
code/addons/docs/src/blocks/controls/Object.tsx
code/**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier with --write flag to format code before committing
Run ESLint with yarn lint:js:cmd to check for linting issues and fix errors before committing
Files:
code/addons/docs/src/blocks/controls/Object.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: normal
- GitHub Check: nx
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (1)
code/addons/docs/src/blocks/controls/Object.tsx (1)
234-235: The uncontrolled component reset pattern usingjsonStringas key and defaultValue is sound.The implementation correctly handles the reset behavior by leveraging React's key-based remounting. When the parent
valueprop changes, the derivedjsonStringchanges, triggering a remount of the textarea with the newdefaultValue. This ensures the component reflects external resets immediately while allowing user edits to accumulate in the uncontrolled textarea state until blur.The pattern handles edge cases correctly:
- User edits while reset occurs are discarded (expected behavior for a reset)
- Mode switching between raw JSON and tree view works correctly since the key ensures clean state transitions
- Multiple rapid resets are efficiently handled by React's reconciliation
- External value updates while focused are captured on the next blur via
updateRaw- Performance is optimized with memoized computations (
jsonString,updateRaw,onForceVisible)This intentionally differs from the controlled textarea approach used in the Text control, allowing JSON edits to be batched on blur rather than updated on every keystroke. The choice to use an uncontrolled component here is appropriate given the need to debounce edits and provide validation feedback before committing changes.
Closes #24316
What I did
Forced JSON mode Object control to re-render when upstream changes value, as the underlying textarea doesn't seem to want to behave in controlled mode, and we can't rely on the ref being mounted to programmatically update it.
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
I could not find a way to automate tests. I've tried setting up an ArgsTable story with a mocked Docs context and a functioning
resetArgsfunction but did not manage.Manual testing
nextDocumentation
ø
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.