chore: convert library from Flow to TypeScript#260
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (21)
💤 Files with no reviewable changes (3)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (10)
📝 WalkthroughWalkthroughMigrates react-resizable from Flow to TypeScript: adds tsconfigs, swaps Babel preset to TypeScript, converts types and components to TS/TSX, updates ESLint/Jest/build scripts, adjusts tests for refactored dimension logic, and removes Flow artifacts and definitions. ChangesFlow-to-TypeScript Conversion
🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
Migrate lib/ and __tests__/ from Flow to TypeScript: - Replace @babel/preset-flow with @babel/preset-typescript - Convert lib/*.js (Flow) to *.ts/*.tsx with explicit types - Add lib/index.ts as a typed entry-point for declaration emit - Drop flow-bin, flow-typed/, .flowconfig - Add tsconfig.json (typecheck), tsconfig.build.json (.d.ts emit), tsconfig.test.json - build.sh now runs babel + tsc --emitDeclarationOnly - ESLint switched to @typescript-eslint/parser for ts/tsx - Tests renamed to .test.tsx / .test.ts, kept JS-style; jest picks them up via babel - package.json gains 'typecheck' script and 'types' field Public runtime API is unchanged. PropTypes runtime validation preserved. Verified: yarn typecheck, yarn lint, yarn test (50 passing), yarn build (emits .js + .d.ts).
STRML
force-pushed
the
2026-05-typescript-conversion
branch
from
f373c31 to
9c19c68
Compare
Packj audit found 1/1 risky dependencies.
|
| Registry | Package | Version | Risks |
|---|---|---|---|
| npm | react-draggable | 4.5.0 |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
lib/propTypes.ts (1)
98-105: 💤 Low valueConsider simplifying the validator signature.
The explicit tuple type
[Record<string, any>, string, string, any, any, any]matches PropTypes' custom validator signature but is brittle. Consider using...args: any[]for better maintainability while preserving the same runtime behavior.♻️ Simpler signature
- height: (...args: [Record<string, any>, string, string, any, any, any]) => { + height: (...args: any[]) => { const [props] = args;🤖 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 `@lib/propTypes.ts` around lines 98 - 105, The height prop-type validator in lib/propTypes.ts uses a brittle explicit tuple type for its arguments; change the parameter signature from (...args: [Record<string, any>, string, string, any, any, any]) to a looser (...args: any[]) in the height function so it remains compatible with PropTypes custom validators but is easier to maintain; keep the internal logic that extracts const [props] = args and the conditional return of (PropTypes.number.isRequired as any)(...args) versus (PropTypes.number as any)(...args) so runtime behavior is unchanged.lib/ResizableBox.tsx (1)
82-94: ⚖️ Poor tradeoffType assertions work around React's defaultProps limitation.
The seven
as anycasts are necessary because TypeScript doesn't understand thatResizable.defaultPropswill provide values for these props. This is a known limitation of TypeScript's support for React'sdefaultPropspattern.For better type safety, consider restructuring the type definitions to make
DefaultPropsfields explicitly optional in thePropstype, or use a more sophisticated pattern like:type PropsWithDefaults<T, D> = Partial<D> & Omit<T, keyof D>;However, this requires significant refactoring and the current approach is pragmatic given React's defaultProps semantics.
🤖 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 `@lib/ResizableBox.tsx` around lines 82 - 94, The seven "as any" casts on props like axis, draggableOpts, handleSize, lockAspectRatio, maxConstraints, minConstraints, resizeHandles and transformScale are hiding a typing mismatch caused by React defaultProps; update the prop types instead of casting: introduce a DefaultProps type listing the defaulted fields and change the component Props to use a utility type (e.g. PropsWithDefaults<T,D> = Partial<D> & Omit<T, keyof D>) or make those fields optional on Props, then update the Resizable/ResizableBox component signature to use that new Props type so the JSX props (axis, draggableOpts, handleSize, lockAspectRatio, maxConstraints, minConstraints, resizeHandles, transformScale) no longer need "as any" casts and onResize (this.onResize) keeps its correct typed signature.
🤖 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 `@lib/propTypes.ts`:
- Around line 98-105: The height prop-type validator in lib/propTypes.ts uses a
brittle explicit tuple type for its arguments; change the parameter signature
from (...args: [Record<string, any>, string, string, any, any, any]) to a looser
(...args: any[]) in the height function so it remains compatible with PropTypes
custom validators but is easier to maintain; keep the internal logic that
extracts const [props] = args and the conditional return of
(PropTypes.number.isRequired as any)(...args) versus (PropTypes.number as
any)(...args) so runtime behavior is unchanged.
In `@lib/ResizableBox.tsx`:
- Around line 82-94: The seven "as any" casts on props like axis, draggableOpts,
handleSize, lockAspectRatio, maxConstraints, minConstraints, resizeHandles and
transformScale are hiding a typing mismatch caused by React defaultProps; update
the prop types instead of casting: introduce a DefaultProps type listing the
defaulted fields and change the component Props to use a utility type (e.g.
PropsWithDefaults<T,D> = Partial<D> & Omit<T, keyof D>) or make those fields
optional on Props, then update the Resizable/ResizableBox component signature to
use that new Props type so the JSX props (axis, draggableOpts, handleSize,
lockAspectRatio, maxConstraints, minConstraints, resizeHandles, transformScale)
no longer need "as any" casts and onResize (this.onResize) keeps its correct
typed signature.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cab75a42-e401-45be-859a-7530d33466c4
⛔ Files ignored due to path filters (3)
__tests__/__snapshots__/Resizable.test.tsx.snapis excluded by!**/*.snap__tests__/__snapshots__/ResizableBox.test.tsx.snapis excluded by!**/*.snapyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (21)
.babelrc.claude/commands/goal.md.flowconfig__tests__/Resizable.test.tsx__tests__/ResizableBox.test.tsx__tests__/ssr.test.tsbuild.sheslint.config.jsflow-typed/npm/jest_v26.x.x.jsjest.config.jslib/Resizable.tsxlib/ResizableBox.tsxlib/index.tslib/propTypes.tslib/utils.jslib/utils.tspackage.jsontsconfig.build.jsontsconfig.jsontsconfig.test.jsonwebpack.config.js
💤 Files with no reviewable changes (3)
- lib/utils.js
- .flowconfig
- flow-typed/npm/jest_v26.x.x.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrate
lib/and__tests__/from Flow to TypeScript. Public runtime API (and PropTypes validation) is unchanged.@babel/preset-flowwith@babel/preset-typescriptlib/*.jsto*.ts/*.tsxwith explicit typeslib/index.tsfor the typed entry-pointflow-bin,flow-typed/,.flowconfig, all// @flowpragmastsconfig.json(typecheck),tsconfig.build.json(declaration emit),tsconfig.test.jsonbuild.shnow runs babel +tsc --emitDeclarationOnlyto produce.js+.d.ts@typescript-eslint/parserfor.ts/.tsx.test.tsx/.test.ts; jest picks them up via babelpackage.jsongains atypecheckscript and atypes: "./build/index.d.ts"fieldWhy now
Flow is no longer well-supported in the React ecosystem and most consumers expect TypeScript declarations. Pinning to
flow-bin@0.153.0(a 2020 build) was holding back tooling upgrades and providing zero value to downstream TS users.Test plan
yarn installyarn typecheck—tsc --noEmitcleanyarn lint— 0 errors (1 pre-existingjest/expect-expectwarning)yarn test— 50/50 tests pass, snapshots regenerated and equivalentyarn build— emitsbuild/{Resizable,ResizableBox,propTypes,utils,index}.{js,d.ts}import {Resizable} from 'react-resizable'Notes
.test.tsxcounterparts. Diff shows the underlying rendered output is unchanged./goalslash command used to scope this work lives at.claude/commands/goal.md.Summary by CodeRabbit
New Features
Chores
Tests