Skip to content

fix: allow lazy loading Draggable and DraggableCore - #825

Open
tevinch wants to merge 1 commit into
react-grid-layout:masterfrom
tevinch:lazy-component-types
Open

tevinch wants to merge 1 commit into
react-grid-layout:masterfrom
tevinch:lazy-component-types

Conversation

@tevinch

@tevinch tevinch commented Sep 28, 2026 •

Copy link
Copy Markdown

Fixes #822.

Direct React.lazy imports now type-check for both Draggable and DraggableCore with React 18 and 19 types, without consumer casts. The derived-state parameter and public constructor accept the same partial props as the component. Both optional propTypes statics use React's component contract, preserving the no-props JSX case and avoiding a generated prop-types declaration dependency. Runtime implementation is unchanged.

Regression coverage includes both lazy imports, minimal JSX, invalid-prop rejection, and a consumer of the built package through the NodeNext export map.

Validation on Node.js 24.19.0 / macOS:

  • make lint passed, including both React type configurations.
  • yarn test: 204 passed.
  • make build passed, including declaration and ESM consumer checks.
  • Fresh packaged consumers passed with React/types 18.3.x and 19.3.0, TypeScript 5.9.3, and skipLibCheck disabled.

Browser drag tests were not run for this type-only change.

Summary by CodeRabbit

  • Bug Fixes
    • Improved TypeScript support for using the draggable components with React.lazy.
    • Added checks to ensure lazy-loaded components accept valid props and flag unsupported axis values or invalid scale types.
    • Updated component prop type declarations to work more consistently with React 18 and generated TypeScript declarations.

@coderabbitai

coderabbitai Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: e020cab9-c6d1-4a86-a476-a3689026280a

📥 Commits

Reviewing files that changed from the base of the PR and between 40829e3 and 929b5a0.

📒 Files selected for processing (5)
  • lib/Draggable.tsx
  • lib/DraggableCore.tsx
  • scripts/verify-build.cjs
  • typings/test.tsx
  • typings/tsconfig.react18.json

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

Draggable’s declarations now accept partial props for state derivation and construction. Draggable and DraggableCore use React’s component propTypes type. Build and typings fixtures check lazy imports and prop validation.

Changes

Component typing compatibility

Layer / File(s) Summary
Component declaration compatibility
lib/Draggable.tsx, lib/DraggableCore.tsx, scripts/verify-build.cjs
Draggable’s state derivation and constructor overload accept partial props. Both components use React.ComponentClass['propTypes']. The build diagnostic recommends that annotation.
Lazy import type checks
scripts/verify-build.cjs, typings/test.tsx, typings/tsconfig.react18.json
The fixtures type-check lazy imports of both components and verify rejection of invalid axis and scale values. The React 18 TypeScript configuration sets module to commonjs.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: strml

Merge Risk: ⚪ Minimal · up to 929b5

This change only adjusts TypeScript declarations so Draggable and DraggableCore can be imported with React.lazy. It adds type checks for this usage and does not change runtime behavior. No merge-blocking risk remains.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 929b5

The changes primarily affect how consumers type-check the components. Existing runtime validation and drag controls remain in place. Direct construction with incomplete props is newly accepted by the types but may fail at runtime; its use by consumers is unknown.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The evidenced exposure is the published component typing contract. The reviewed changes do not establish a new privileged sink, service boundary, or expansion of drag-event authority.

Trust Boundaries and Controls

  • inferred — Normal React rendering still passes through the component defaults, runtime prop validators, and DraggableCore interaction controls. Direct class construction does not receive React's default-prop resolution, but no security-sensitive consumer path through it was evidenced.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: enabling direct lazy loading of both Draggable and DraggableCore.
Linked Issues check ✅ Passed Issue #822 requires Draggable to satisfy React's ComponentType contract for direct React.lazy imports without a consumer cast. lib/Draggable.tsx now types getDerivedStateFromProps and the pu…
Out of Scope Changes check ✅ Passed The changed files support the linked typing objective. The DraggableCore propTypes annotation preserves the same React component contract for the exported component. The build verification updates…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 4…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Warning

Some tools did not complete. Review the errors below.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

lib/Draggable.tsx

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.

lib/DraggableCore.tsx

ESLint skipped: the matched ESLint configuration already failed (missing-dependency).

scripts/verify-build.cjs

ESLint skipped: the matched ESLint configuration already failed (missing-dependency).

  • 1 others

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

React.lazy(() => import('react-draggable')) fails TypeScript: typeof Draggable is not assignable to ComponentType

1 participant