Skip to content

Add TypeScript type definitions from DefinitelyTyped#12

Merged
alexeyr-ci2 merged 3 commits intomasterfrom
alexeyr/add-types
Feb 13, 2026
Merged

Add TypeScript type definitions from DefinitelyTyped#12
alexeyr-ci2 merged 3 commits intomasterfrom
alexeyr/add-types

Conversation

@alexeyr-ci2
Copy link

@alexeyr-ci2 alexeyr-ci2 commented Feb 12, 2026

Summary

  • Copy index.d.ts from @types/shakacode__recompose into the package so types ship with the library
  • Add "types": "index.d.ts" to package.json

Test plan

  • Verify TypeScript consumers can import types without @types/shakacode__recompose

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added comprehensive TypeScript typings for the recompose public API, including observable integrations and per-module type surfaces for better IDE support.
  • Documentation

    • Updated API docs and examples to TypeScript, with generic signatures for HOCs, state helpers, lifecycle, and observable utilities.
  • Chores

    • Published typings entry in package metadata so TypeScript tooling can discover the declarations.

@coderabbitai
Copy link

coderabbitai bot commented Feb 12, 2026

Walkthrough

Adds a complete TypeScript declaration surface for the @shakacode/recompose package, wires package.json types to index.d.ts, and updates API documentation examples to TypeScript. All edits are type declarations and docs; no runtime code changes. (≤50 words)

Changes

Cohort / File(s) Summary
TypeScript Declarations
src/packages/recompose/index.d.ts
Introduces a comprehensive module declaration (~738 lines) exposing HOC typings, enhancer types, state/reducer/lifecycle typings, observable/stream integrations, helper types, and per-submodule declarations. No runtime changes.
Package Configuration
src/packages/recompose/package.json
Adds "types": "index.d.ts" to publish TypeScript declarations.
Documentation
docs/API.md
Converts many examples and code fences to TypeScript and updates signatures in docs (generics, observable helpers). No behavior changes.
Observable Config Modules
@shakacode/recompose/*ObservableConfig (declared within index.d.ts)
Adds declarations and default-export wiring for multiple observable-config modules (rxjs, rxjs4, most, xstream, bacon, kefir).
Manifest
manifest_file
Large addition recorded reflecting new declarations and metadata changes (+738/-0 lines).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐇 I hopped through types with eager paws,

Mapping props and mending flaws.
Generics twinkle in my lair,
Streams well-typed, and tidy care.
Recompose dressed up fine and fair.

🚥 Pre-merge checks | ✅ 3 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Merge Conflict Detection ⚠️ Warning ❌ Merge conflicts detected (2 files):

⚔️ docs/API.md (content)
⚔️ src/packages/recompose/package.json (content)

These conflicts must be resolved before merging into master.
Resolve conflicts locally and push changes to this branch.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and directly describes the main change: adding TypeScript type definitions to the package by integrating definitions from DefinitelyTyped.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch alexeyr/add-types
⚔️ Resolve merge conflicts (beta)
  • Auto-commit resolved conflicts to branch alexeyr/add-types
  • Create stacked PR with resolved conflicts
  • Post resolved changes as copyable diffs in a comment

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
src/packages/recompose/index.d.ts (4)

377-380: Clean up the // ??? uncertainty marker.

This leftover comment from the DefinitelyTyped source suggests uncertainty about the return type. Since this is now shipping as the canonical type definition with the package, these markers should be resolved or removed.

The return type React.ComponentClass<any> is reasonable for nest() — it returns a class component that passes all props through to the outermost nested component.

Proposed cleanup
     export function nest(
         ...Components: Array<string | Component<any>>
-    ): React.ComponentClass<any>; // ???
+    ): React.ComponentClass<any>;

394-397: Clean up the second // ??? marker on componentFromStream.

Same as nest() above — this uncertainty comment should be removed before shipping.

Proposed cleanup
     export function componentFromStream<TProps>(
         propsToReactNode: mapper<Subscribable<TProps>, Subscribable<React.ReactNode>>,
-    ): Component<TProps>; // ???
+    ): Component<TProps>;

335-343: Remove commented-out compose overloads.

These commented-out alternatives are dead code carried over from DefinitelyTyped. If they're needed as reference, a code comment explaining the tradeoff would be more helpful than commented-out type signatures.

Proposed cleanup
     export function compose<TInner, TOuter>(
         ...functions: Function[]
     ): ComponentEnhancer<TInner, TOuter>;
-    // export function compose<TOuter>(
-    //     ...functions: Array<Function>
-    // ): ComponentEnhancer<any, TOuter>;
-    // export function compose(
-    //     ...functions: Array<Function>
-    // ): ComponentEnhancer<any, any>;

116-136: stateProps type name uses lowercase — consider StateProps for consistency.

The internal types stateProps (line 117) and reducerProps (line 153) use lowercase names, while other internal types like StateHandler, StateHandlerMap, and StateUpdaters use PascalCase. This inconsistency is inherited from DefinitelyTyped, so it's a minor nit.


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 and usage tips.

@justin808
Copy link
Member

@claude review this PR

@claude
Copy link

claude bot commented Feb 13, 2026

Claude finished @justin808's task in 1m 26s —— View job


PR Review: TypeScript Type Definitions

  • Read changed files (index.d.ts and package.json)
  • Review TypeScript type definitions for correctness
  • Verify types align with implementation
  • Provide comprehensive review feedback

Summary

This PR successfully adds TypeScript type definitions from DefinitelyTyped to the recompose package. The implementation is solid overall, with the types correctly copied from the @types/shakacode__recompose package and the package.json properly configured.

Issues Found

1. Critical: Missing exports in type definitions (src/packages/recompose/index.d.ts)

The type definitions declare createEagerElement and createEagerFactory (lines 370-381), but these functions are not exported from the actual JavaScript implementation in src/packages/recompose/index.js. This will cause TypeScript users to get type errors when trying to import these functions.

Impact: Users will see type definitions suggesting these functions exist, but they'll get runtime errors when trying to use them.

Recommendation: Either remove these type declarations from the .d.ts file, or verify if these functions should be added to the JavaScript implementation. Based on my review, they don't exist in the codebase.

2. Typo: "TOutter" instead of "TOuter" (throughout the file)

The type definitions consistently use TOutter (with double 't') instead of the more standard TOuter. Examples:

  • Line 11: type mapper<TInner, TOutter>
  • Line 31: interface ComponentEnhancer<TInner, TOutter>
  • And many more throughout the file

Impact: Low - this is a cosmetic issue and doesn't affect functionality, but it's inconsistent with TypeScript naming conventions.

Recommendation: Consider renaming all instances to TOuter in a follow-up PR for consistency with standard TypeScript conventions.

3. Incorrect withState type signature (line 126)

The withState type definition at line 126 has an incorrect type signature:

& { [stateUpdateName in TStateUpdaterName]: (state: TState) => TState };

This suggests the state updater returns TState, but looking at the implementation in src/packages/recompose/withState.js:19-26, the updater accepts either a function (state) => newState OR a direct value. The current type is too restrictive.

Impact: Users won't be able to properly type check their state updater calls.

Recommendation: The type should be:

& { [stateUpdateName in TStateUpdaterName]: (state: TState | ((prevState: TState) => TState), callback?: () => void) => void };

Positive Aspects

✅ Package.json correctly configured with "types": "index.d.ts" (line 14)
✅ Comprehensive coverage of all exported functions from the implementation
✅ Good documentation with links to the API docs
✅ Support for individual module imports (e.g., @shakacode/recompose/mapProps)
✅ Observable configs properly typed for RxJS, Most, Bacon, Kefir, and xstream

Test Plan Verification

The test plan includes:

  • Verify TypeScript consumers can import types without @types/shakacode__recompose

This is a good test, but I recommend also testing:

  • Verify the missing createEagerElement and createEagerFactory don't cause issues
  • Test that withState updater function works with both function and value forms

Recommendation

BEFORE MERGING: Fix issue #1 (remove createEagerElement and createEagerFactory declarations) as this will cause actual TypeScript errors for users.

Issues #2 and #3 could be addressed in follow-up PRs if desired, as they're lower priority.

Fix critical issue #1 →


Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In `@src/packages/recompose/index.d.ts`:
- Around line 423-427: The type ObservableConfig is declared but not exported,
causing imports (e.g., in observable-config submodules) to fail; export the type
by adding an export for ObservableConfig so external modules can import it
(ensure the declaration for ObservableConfig is changed to an exported type and
keep the existing setObservableConfig signature referring to that exported
type); update any related declarations that import ObservableConfig to use the
exported symbol as-is.
- Around line 476-482: Add a missing module declaration for
"@shakacode/recompose/flydObservableConfig" matching the pattern used by other
observable configs (e.g., the kefir declaration): import the ObservableConfig
type, declare a const flydConfig: ObservableConfig, and export it as the default
export (i.e., export default flydConfig) so the type definition for the flyd
config exists alongside the other observable modules.
- Around line 345-354: Update the type signatures to match the JS
implementation: change getDisplayName to accept Component<any> | string |
undefined and to return string | undefined (so it returns undefined for falsy
input and passes through string inputs), and change wrapDisplayName to accept
Component<any> | string | undefined (so callers can pass a string component
name) while keeping its return type as string; update the exported function
declarations for getDisplayName and wrapDisplayName accordingly.
🧹 Nitpick comments (2)
docs/API.md (1)

898-908: Documentation uses higher-kinded type notation (TStream<T>) that isn't valid TypeScript.

The componentFromStreamWithConfig, mapPropsStreamWithConfig, createEventHandlerWithConfig, and setObservableConfig signatures in the docs use TStream<T> as a generic type constructor (e.g., (observable: Observable<T>) => TStream<T>). TypeScript doesn't support higher-kinded types, and the actual index.d.ts correctly works around this with any. Since this is API documentation meant for human consumption, this is acceptable as pseudocode, but it could confuse TypeScript users who try to use these signatures literally.

Consider adding a brief note (e.g., "Signatures below use pseudocode; see the .d.ts file for exact TypeScript types") near the Observable utilities section header.

Also applies to: 977-988, 1019-1031, 1037-1042

src/packages/recompose/index.d.ts (1)

382-386: hoistStatics signature is overly restrictive — only accepts InferableComponentEnhancer.

The current overload only works with InferableComponentEnhancer<TProps>, but hoistStatics is commonly used with any HOC including InferableComponentEnhancerWithProps and ComponentEnhancer. This will force unnecessary type assertions at call sites.

Proposed overloads
     export function hoistStatics<TProps>(
         hoc: InferableComponentEnhancer<TProps>,
         blacklist?: { [key: string]: boolean },
     ): InferableComponentEnhancer<TProps>;
+    export function hoistStatics<TInner, TOuter>(
+        hoc: InferableComponentEnhancerWithProps<TInner, TOuter>,
+        blacklist?: { [key: string]: boolean },
+    ): InferableComponentEnhancerWithProps<TInner, TOuter>;

Comment on lines +345 to +354
// getDisplayName: https://github.com/shakacode/recompose/blob/master/docs/API.md#getDisplayName
export function getDisplayName(
component: Component<any>,
): string;

// wrapDisplayName: https://github.com/shakacode/recompose/blob/master/docs/API.md#wrapDisplayName
export function wrapDisplayName(
component: Component<any>,
wrapperName: string,
): string;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

getDisplayName and wrapDisplayName types don't match the JS implementation.

Per getDisplayName.js, the function accepts string values (returns them as-is) and returns undefined for falsy input. The current type only accepts Component<any> and always returns string, which will cause type errors for valid call sites like getDisplayName('div').

Similarly, wrapDisplayName accepts a string component name per the implementation.

Proposed fix
     export function getDisplayName(
-        component: Component<any>,
+        component: Component<any> | string,
     ): string;

     export function wrapDisplayName(
-        component: Component<any>,
+        component: Component<any> | string,
         wrapperName: string,
     ): string;
🤖 Prompt for AI Agents
In `@src/packages/recompose/index.d.ts` around lines 345 - 354, Update the type
signatures to match the JS implementation: change getDisplayName to accept
Component<any> | string | undefined and to return string | undefined (so it
returns undefined for falsy input and passes through string inputs), and change
wrapDisplayName to accept Component<any> | string | undefined (so callers can
pass a string component name) while keeping its return type as string; update
the exported function declarations for getDisplayName and wrapDisplayName
accordingly.

alexeyr and others added 3 commits February 13, 2026 08:21
Copy index.d.ts from @types/shakacode__recompose into the package
so types ship with the library instead of requiring a separate
@types install.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Remove createEagerElement and createEagerFactory (not in JS exports)
- Remove custom Omit type (built into TypeScript since 3.5)
- Fix TOutter/outterName typos to TOuter/outerName throughout
- Fix withState updater type to accept value or function, return void
- Fix branch to share TInner/TOuter across both enhancers
- Fix renderComponent return type to preserve TProps
- Fix withContext getChildContext return type to use TContext
- Make createSink generic over TProps

Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Convert Flow syntax (?Type, ?param) to TypeScript (Type?, param?)
- Add descriptive type parameters (TState, TAction, TStream, TValue)
  with explanatory comments to all generic signatures
- Add type parameters to withState, withStateHandlers, withContext,
  createSink to match withReducer/createEventHandler pattern
- Fix withReducer dispatch callback type to receive new state
- Change code blocks from js to ts for typed signatures

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@alexeyr-ci2
Copy link
Author

@coderabbitai Review.

@alexeyr-ci2
Copy link
Author

@claude Review.

@coderabbitai
Copy link

coderabbitai bot commented Feb 13, 2026

@alexeyr-ci2, I'll review this PR for you. I'll analyze the TypeScript type definitions, package.json changes, and documentation updates.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@claude
Copy link

claude bot commented Feb 13, 2026

Claude finished @alexeyr-ci2's task in 1m 39s —— View job


PR Review

  • Read changed files
  • Review TypeScript type definitions
  • Check previous review comments
  • Provide comprehensive feedback

Summary

This PR successfully adds TypeScript type definitions from DefinitelyTyped to the recompose package. The implementation is solid and all critical issues from CodeRabbit's previous review have been addressed in commits 0ec7e95 and 70e74e0.

Issues Previously Raised and Resolved

All three actionable comments from CodeRabbit's latest review have been fixed:

  1. ObservableConfig export (fixed in 70e74e0) - The type is now properly exported on line 427 of index.d.ts
  2. Missing flydObservableConfig module (fixed in 70e74e0) - Module declaration added at lines 488-495
  3. getDisplayName and wrapDisplayName type signatures (fixed in 0ec7e95) - Both now correctly accept Component<any> | string parameters (lines 346-354)

Current State Review

After reviewing the current state of the code, I found the implementation to be correct and well-structured:

Strengths:

  • ✅ Package.json correctly configured with "types": "index.d.ts" (line 14)
  • ✅ All functions exported from index.js have corresponding type declarations
  • ✅ Type signatures accurately match JavaScript implementation behavior
  • ✅ Comprehensive coverage including observable integrations for RxJS, Most, Bacon, Kefir, xstream, and flyd
  • ✅ Support for individual module imports (e.g., @shakacode/recompose/mapProps)
  • ✅ All TypeScript-specific improvements applied (TOuter typo fixed, removed custom Omit type, etc.)
  • ✅ CI checks passing

Verified Type Accuracy:
I cross-checked several implementations against their type definitions:

  • withState (lines 117-136) - Type correctly represents that updater accepts TState | ((prevState: TState) => TState) and returns void
  • getDisplayName (lines 346-348) - Type correctly accepts strings and returns string
  • wrapDisplayName (lines 350-354) - Type correctly accepts strings as component names
  • All exports in index.js match the type declarations

Minor Observations (Non-blocking)

These are cosmetic and inherited from DefinitelyTyped - not issues that need fixing:

  1. Commented-out code (lines 338-343) - Alternative compose overloads are commented out. This is fine as reference material.
  2. // ??? markers removed - Good! The previous uncertainty markers on nest() and componentFromStream() have been cleaned up.
  3. Type naming convention - Types like stateProps (line 117) use lowercase while others like StateHandler use PascalCase. This is inherited from DefinitelyTyped and doesn't affect functionality.

Recommendation

✅ APPROVE - This PR is ready to merge. All critical issues have been addressed, type definitions accurately match the JavaScript implementation, and the package is properly configured to ship types with the library.

Test Plan

The test plan includes:

  • Verify TypeScript consumers can import types without @types/shakacode__recompose

I recommend also testing:

  • Verify individual module imports work (e.g., import mapProps from '@shakacode/recompose/mapProps')
  • Verify observable config modules can be imported
  • Test that common HOC patterns type-check correctly

@alexeyr-ci2 alexeyr-ci2 merged commit ff48b17 into master Feb 13, 2026
2 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Feb 13, 2026
4 tasks
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.

3 participants