10000 fix(prop): preserve generic types when used with indexBy by Copilot · Pull Request #1204 · remeda/remeda · GitHub
[go: up one dir, main page]
Skip to content

fix(prop): preserve generic types when used with indexBy #1204

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
Copilot wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

fix(prop): preserve generic types when used with indexBy #1204

Copilot wants to merge 3 commits into from
+41 −4

Conversation

Copy link
Contributor
Copilot AI commented Oct 2, 2025 •
edited
Loading

Problem

Using prop with generic constrained types would lose type information when passed to functions like indexBy, making it impossible to index the resulting record. This was a regression introduced in v2.27.0.

// This worked fine
function foo<T extends { id: string }>(data: T[], key: T["id"]): T {
  return indexBy(data, ({ id }) => id)[key];
}

// But this failed with TypeScript error
function bar<T extends { id: string }>(data: T[], key: T["id"]): T {
  return indexBy(data, prop("id"))[key];
  // Error: Type 'T["id"]' cannot be used to index type 
  // 'BoundedPartial<Record<NoInfer<Prop<T, "id">>, T>>'
}

Root Cause

The single-key data-last overload of prop was returning NoInfer<Prop<T, Key>>:

export function prop<T extends NonPropertyKey, Key extends KeysOfUnion<T>>(
  key: Key,
): (data: T) => NoInfer<Prop<T, Key>>;

When used with generic types, Prop<T, "id"> (even though structurally equivalent to T["id"]) was not recognized by TypeScript as compatible with T["id"] for Record indexing purposes. This is because the Prop helper uses distributive conditional types which prevent TypeScript from simplifying the type in generic contexts.

Solution

Removed the problematic single-key data-last overload and rely on the existing fallback overload that returns T[K] directly:

export function prop<K extends PropertyKey>(
  key: K,
): <T extends Partial<Record<K, unknown>>>(data: T) => T[K];

This fallback overload:

  • Returns T[K] directly (compatible with Record indexing)
  • Preserves generic type information
  • Maintains all existing functionality for unions and arrays

Impact

The fix is minimal and surgical - only one overload was removed. All existing tests pass, and the new test case verifies the fix works correctly:

function testWithProp<T extends { id: string }>(
  data: ReadonlyArray<T>,
  key: T["id"],
): T | undefined {
  return indexBy(data, prop("id"))[key]; // ✅ Now works!
}

Note: The return type is T | undefined (not T) because indexBy returns a BoundedPartial record, which correctly models that not all keys may be present.

Testing

  • ✅ All 1947 existing type tests pass
  • ✅ All 20 runtime tests pass
  • ✅ New test case added to prevent regression
  • ✅ Verified with original issue reproduction

Fixes #[issue number]

Original prompt

This section details on the original issue you should resolve

<issue_title>prop: breaking generic index types</issue_title>
<issue_description>This seems to be a regression introduced in v2.27.0 (cb7d611).

Using prop, the generic gets lost in index types.

Image

A workaround is to use ({id}) => id instead of prop('id').

Reproduction: https://www.typescriptlang.org/play/?noUncheckedIndexedAccess=true&declaration=false&noImplicitOverride=false&noPropertyAccessFromIndexSignature=false#code/JYWwDg9gTgLgBAKjgQwM5wEpwGZQiOAcigFMQSATZQgKBuwFcA7AYxmAibhGWCYB4WnVPAAqcEgA8YJJhXQBvYBQBccEVD4BzAL4A+ABRUYyNaIDaAXQCUcBTTiO4QpiLjHkAIQCeASQoAjHAAvJgAdHwUUj5GyCYANOFgeGAGhMqE1tYOTi5uHj7+AEwh4ZHR3rEJcAYKcMpwOrbBevUU2TmOefAQDDBBoQV+geaEhCjoFukUhJaWnc7CPX0lg3Few0Wj42hwUxlzNDpAA

</issue_description>

Comments on the Issue (you are @copilot in this section)

@eranhirsch I started looking into this last week. This isn't trivial to fix, but I believe it's solvable. In the meantime, for simple prop access, the workaround of using `({ myProp }) => myProp` instead of `prop("myProp")` is the only way.

Minimal repro:

// Works
function foo<T extends { id: string }>(data: T[], key: T["id"]): T {
  return indexBy(data, ({ id }) => id)[key];
}

// Doesn't work:
function bar<T extends { id: string }>(data: T[], key: T["id"]): T {
  return indexBy(data, prop("id"))[key];
}

https://www.typescriptlang.org/play/?#code/JYWwDg9gTgLgBAbzsAdgEwKYA8BCBPAGjjCgjDgF84AzUkOAIigxAzQEMGBuAKB4Hp+cAOrQA1gGce1AK4oAxjGAQUNCBAA8AFTjYYGdBMTI0ALjgSYUVAHNKAPgAUHGO3NaA2gF0iYjHncPBmA0Bi8ASndEHjg4ZhgZKFVUTFw8Z3ZXIkckEMpwuABeexNwjz88L14KPkE4ABEIDAkUAHJ4AHdxU2k5RWVVACN2KG1dLH1DYxDzS2sUOwonFzc4Tx84CsDg0IiohBi4jASk5HRsfAys4lIwRx2GcLKKqp4aoA</comment_new>

Fixes #1174

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@bolt-new-by-stackblitz
Copy link
bolt-new-by-stackblitz bot commented Oct 2, 2025

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@netlify
Copy link
netlify bot commented Oct 2, 2025 •
edited
Loading

✅ Deploy Preview for trusting-lumiere-9c7fad ready!

Name Link
🔨 Latest commit 5f0a2a5
🔍 Latest deploy log https://app.netlify.com/projects/trusting-lumiere-9c7fad/deploys/68de7bdca05037000879e47a
😎 Deploy Preview https://deploy-preview-1204--trusting-lumiere-9c7fad.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copilot AI changed the title [WIP] prop: breaking generic index types Fix prop: preserve generic types when used with indexBy Oct 2, 2025
Copilot AI requested a review from eranhirsch October 2, 2025 13:20
Copilot finished work on behalf of eranhirsch October 2, 2025 13:20 8000
@codecov
Copy link
codecov bot commented Oct 2, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (9086669) to head (5f0a2a5).

Additional details and impacted files 
@@            Coverage Diff            @@
##              main     #1204   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          174       174           
  Lines         2918      2918           
  Branches       959       959           
=========================================
  Hits          2918      2918

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pkg-pr-new
Copy link
pkg-pr-new bot commented Oct 2, 2025

@remeda/stackblitz-template

npm i https://pkg.pr.new/remeda@1204

commit: 5f0a2a5

@eranhirsch eranhirsch changed the title Fix prop: preserve generic types when used with indexBy fix(prop): preserve generic types when used with indexBy Oct 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eranhirsch eranhirsch Awaiting requested review from eranhirsch

Assignees

@eranhirsch eranhirsch

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

prop: breaking generic index types

2 participants

@eranhirsch
0