refactor(draft-js-editor): migrate DraftJSEditor from Flow to TypeScript - #4730
Conversation
WalkthroughAdds a typed DraftJS editor with accessibility wiring, event handling, labels, disabled mode, and error tooltips. Exports it through component barrels, adds DraftJS typings, and updates its story and tests. ChangesDraftJS editor
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Consumer
participant DraftJSEditor
participant DraftEditor
participant Tooltip
Consumer->>DraftJSEditor: Pass editorState and callbacks
DraftJSEditor->>DraftEditor: Render editor and accessibility props
DraftEditor->>DraftJSEditor: Emit change, focus, blur, or return
DraftJSEditor->>Consumer: Forward event or updated editorState
DraftJSEditor->>Tooltip: Show error.message when error exists
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/draft-js-editor/__tests__/DraftJSEditor.test.tsx`:
- Around line 61-62: Update the test’s onChange invocation to pass a valid
EditorState object, and type the callback according to the component’s actual
EditorState contract instead of casting it to () => void. Assert or preserve the
existing forwarding expectations so the test verifies the received EditorState
rather than allowing undefined.
In `@src/components/draft-js-editor/DraftJSEditor.js.flow`:
- Around line 37-59: Update DraftJSEditor’s Flow contract by removing the
isFocused default from defaultProps and changing handleBlur and the onBlur prop
typing to use the focus/blur event type emitted by DraftJS. Ensure handleBlur
forwards that event unchanged to onBlur, while leaving handleChange’s
EditorState contract intact.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a885fd77-9045-4887-a347-7f34937527d1
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (8)
package.jsonsrc/components/draft-js-editor/DraftJSEditor.js.flowsrc/components/draft-js-editor/DraftJSEditor.stories.tsxsrc/components/draft-js-editor/DraftJSEditor.tsxsrc/components/draft-js-editor/__tests__/DraftJSEditor.test.tsxsrc/components/draft-js-editor/index.js.flowsrc/components/draft-js-editor/index.tssrc/components/index.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/draft-js-editor/__tests__/DraftJSEditor.test.tsx`:
- Around line 61-62: Update the test’s onChange invocation to pass a valid
EditorState object, and type the callback according to the component’s actual
EditorState contract instead of casting it to () => void. Assert or preserve the
existing forwarding expectations so the test verifies the received EditorState
rather than allowing undefined.
In `@src/components/draft-js-editor/DraftJSEditor.js.flow`:
- Around line 37-59: Update DraftJSEditor’s Flow contract by removing the
isFocused default from defaultProps and changing handleBlur and the onBlur prop
typing to use the focus/blur event type emitted by DraftJS. Ensure handleBlur
forwards that event unchanged to onBlur, while leaving handleChange’s
EditorState contract intact.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a885fd77-9045-4887-a347-7f34937527d1
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (8)
package.jsonsrc/components/draft-js-editor/DraftJSEditor.js.flowsrc/components/draft-js-editor/DraftJSEditor.stories.tsxsrc/components/draft-js-editor/DraftJSEditor.tsxsrc/components/draft-js-editor/__tests__/DraftJSEditor.test.tsxsrc/components/draft-js-editor/index.js.flowsrc/components/draft-js-editor/index.tssrc/components/index.ts
🛑 Comments failed to post (1)
src/components/draft-js-editor/DraftJSEditor.js.flow (1)
37-59: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash set -euo pipefail rg -n -C2 'isFocused|handleBlur\s*=|onBlur:' \ src/components/draft-js-editor/DraftJSEditor.tsx \ src/components/draft-js-editor/DraftJSEditor.js.flowRepository: box/box-ui-elements
Length of output: 2097
🏁 Script executed:
#!/usr/bin/env bash set -euo pipefail printf '--- package copy:flow references ---\n' rg -n '"copy:flow"|"flow"' package.json package-lock.json yarn.lock pnpm-lock.yaml 2>/dev/null || true printf '\n--- relevant draft-js-editor files ---\n' sed -n '1,90p' src/components/draft-js-editor/DraftJSEditor.tsx printf '\n' sed -n '1,90p' src/components/draft-js-editor/DraftJSEditor.js.flow printf '\n--- imports for DraftJS type annotations ---\n' sed -n '1,30p' src/components/draft-js-editor/DraftJSEditor.js.flowRepository: box/box-ui-elements
Length of output: 248
🏁 Script executed:
#!/usr/bin/env bash set -euo pipefail printf '%s\n' '--- package copy:flow references ---' rg -n '"copy:flow"|"flow"' package.json package-lock.json yarn.lock pnpm-lock.yaml 2>/dev/null || true printf '%s\n' '' printf '%s\n' '--- relevant draft-js-editor files ---' sed -n '1,90p' src/components/draft-js-editor/DraftJSEditor.tsx printf '%s\n' '' sed -n '1,90p' src/components/draft-js-editor/DraftJSEditor.js.flow printf '%s\n' '' printf '%s\n' '--- imports and relevant type annotations in flow file ---' sed -n '1,80p' src/components/draft-js-editor/DraftJSEditor.js.flowRepository: box/box-ui-elements
Length of output: 8652
🌐 Web query:
Draft.js Editor onBlur event API EditorState parameter callback event💡 Result:
The Draft.js Editor component's onBlur prop callback does not receive an EditorState parameter; its signature is defined as (e: SyntheticFocusEvent) => void [1][2][3]. When the onBlur event is triggered, the Draft.js internal logic (specifically the editOnBlur function) handles the event, updates the selection state to reflect that the editor no longer has focus, and subsequently invokes your provided onBlur prop with the native synthetic focus event [4]. Because the onBlur callback does not receive the EditorState, if you need to access the current EditorState at the time of the blur event, you must rely on the state held in your component's scope (e.g., this.state.editorState or a React useState hook variable) [5][6]. Common considerations for onBlur implementations: - State Access: Since the callback only provides the event, you should reference your current component state directly to perform any operations based on the EditorState at the time of blurring [6]. - Synchronization: Developers often use the getSelection.getHasFocus method within the onChange handler to track focus changes effectively, as using setState inside onBlur can sometimes cause conflicts with the editor's update cycle [5][6]. - Preserve Selection: By default, Draft.js clears the DOM selection on blur. You can use the preserveSelectionOnBlur prop if you need to maintain native browser selection behavior [7][8].
Citations:
- 1: https://github.com/facebookarchive/draft-js/blob/81cc54b908a7463409ce80c785585a65b3646340/docs/APIReference-Editor.md
- 2: https://www.draftail.org/docs/next/draft-js/api-reference/editor
- 3: https://draftjs.org/docs/api-reference-editor
- 4: https://app.unpkg.com/@wix/design-system@1.302.0/files/dist/node_modules/draft-js/lib/editOnBlur.js.flow
- 5: facebookarchive/draft-js#923
- 6: facebookarchive/draft-js#726
- 7: https://app.unpkg.com/@wix/design-system@1.302.0/files/dist/node_modules/draft-js/lib/DraftEditorProps.js.flow
- 8: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/draft-js/index.d.ts
Synchronize the published Flow compatibility contract.
copy:flowpreservesisFocused: falseand declareshandleBluras acceptingEditorStatewhile callingonBlur(editorState), but DraftJS invokesonBlurwith a focus event. Remove this stale default prop and typehandleBlur/onBluraround the blur event/prop so Flow consumers match the runtime contract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/draft-js-editor/DraftJSEditor.js.flow` around lines 37 - 59, Update DraftJSEditor’s Flow contract by removing the isFocused default from defaultProps and changing handleBlur and the onBlur prop typing to use the focus/blur event type emitted by DraftJS. Ensure handleBlur forwards that event unchanged to onBlur, while leaving handleChange’s EditorState contract intact.
59d3b63 to
34b634f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/draft-js-editor/DraftJSEditor.tsx`:
- Around line 20-47: Replace the generic Function and object types in
DraftJSEditorProps with concrete callback signatures and structured types
matching their usage in DraftJSEditor: type onBlur/onFocus with the expected
React synthetic event, onChange with its editor-state callback parameters,
onReturn with its return-handler contract, error with an optional message field,
and inputProps as a string-keyed prop record. Remove the corresponding casts in
the component implementation, including those around error, inputProps, and
onFocus.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 09e792f8-f510-4b70-ba93-3dd2cfcc06c9
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (8)
package.jsonsrc/components/draft-js-editor/DraftJSEditor.js.flowsrc/components/draft-js-editor/DraftJSEditor.stories.tsxsrc/components/draft-js-editor/DraftJSEditor.tsxsrc/components/draft-js-editor/__tests__/DraftJSEditor.test.tsxsrc/components/draft-js-editor/index.js.flowsrc/components/draft-js-editor/index.tssrc/components/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/draft-js-editor/DraftJSEditor.stories.tsx
34b634f to
850bb6e
Compare
Merge Queue Status
This pull request spent 12 seconds in the queue, including 1 second running CI. Required conditions to merge
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/draft-js-editor/DraftJSEditor.js.flow (1)
21-35: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winFlow entrypoint lacks a props type export, unlike the TS entrypoint.
index.tsexportsDraftJSEditorPropsalongside the default, but the Flow shims only export the default component, so Flow consumers can't import an equivalent typed props contract.
src/components/draft-js-editor/DraftJSEditor.js.flow#L21-L35: export thePropstype (e.g.export type Props = {...}) so it can be re-exported.src/components/draft-js-editor/index.js.flow#L1-L2: addexport type { Props } from './DraftJSEditor';to mirrorindex.ts'sDraftJSEditorPropsexport.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/draft-js-editor/DraftJSEditor.js.flow` around lines 21 - 35, The Flow entrypoint must expose the editor props contract like the TypeScript entrypoint. In src/components/draft-js-editor/DraftJSEditor.js.flow lines 21-35, export the Props type; in src/components/draft-js-editor/index.js.flow lines 1-2, re-export Props from DraftJSEditor so Flow consumers can import it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/components/draft-js-editor/DraftJSEditor.js.flow`:
- Around line 21-35: The Flow entrypoint must expose the editor props contract
like the TypeScript entrypoint. In
src/components/draft-js-editor/DraftJSEditor.js.flow lines 21-35, export the
Props type; in src/components/draft-js-editor/index.js.flow lines 1-2, re-export
Props from DraftJSEditor so Flow consumers can import it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 72a8cd20-4927-4702-84b4-73b8830b926c
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (8)
package.jsonsrc/components/draft-js-editor/DraftJSEditor.js.flowsrc/components/draft-js-editor/DraftJSEditor.stories.tsxsrc/components/draft-js-editor/DraftJSEditor.tsxsrc/components/draft-js-editor/__tests__/DraftJSEditor.test.tsxsrc/components/draft-js-editor/index.js.flowsrc/components/draft-js-editor/index.tssrc/components/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- src/components/draft-js-editor/DraftJSEditor.stories.tsx
3 participants
Convert DraftJSEditor component to TypeScript
This PR converts
src/components/draft-js-editorfrom JavaScript with Flow to TypeScript.Changes
DraftJSEditor.jstoDraftJSEditor.tsxwith exportedDraftJSEditorPropsinterfaceindex.js, Storybook stories, and unit tests to TypeScript.js.flowfiles for backward compatibilityDraftJSEditorexports tosrc/components/index.ts@types/draft-jsisFocuseddefault propTesting
yarn lint:ts, ESLint, andflow checkpassComponents/DraftJSEditorstory builds successfullySummary by CodeRabbit