Skip to content
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.

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

Expanded function return type for certain mapped types in compiled output causes index access error #55049

Open
ssalbdivad opened this issue Jul 17, 2023 · 1 comment Β· May be fixed by #54759
Open
Assignees
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue Rescheduled This issue was previously scheduled to an earlier milestone

Comments

@ssalbdivad
Copy link

ssalbdivad commented Jul 17, 2023

Bug Report

πŸ”Ž Search Terms

index access function return compiled .d.ts expansion

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

type evaluate<t> = { [k in keyof t]: t[k] } & unknown

export type entryOf<o> = evaluate<
    { [k in keyof o]-?: [k, o[k] & ({} | null)] }[o extends readonly unknown[]
        ? keyof o & number
        : keyof o]
>

export type entriesOf<o extends object> = evaluate<entryOf<o>[]>

export const entriesOf = <o extends object>(o: o) =>
    Object.entries(o) as entriesOf<o>

πŸ™ Actual behavior

TS source has no errors, compiled .d.ts has errors

πŸ™‚ Expected behavior

Either TS source should have errors, or .d.ts should not have errors

Addendum

This resulted in a blocking issue for an ArkType user who wasn't able to enable skipLibCheck in their CI environment.

Though the repro conditions are somewhat specific, issues like this can cause big problems for library authors, as I don't think it is commonplace to check build output for errors after TSC is successful.

Although in this specific case a faulty return type expansion seems to be at fault, in general I'd love if the default on skipLibCheck could be revisited. As I understand based on a conversation with @Andarist, individual TSConfig settings like exactOptionalPropertyTypes can affect whether some dependency blocks your build if skipLibChecks is not manually enabled.

Perf considerations aside, while I understand how it could be useful to know if the types of one of your dependencies is doing something unexpected, there's no feasible way for libraries to avoid these types of problems without checking their types under multiple combinations of TSConfig settings. Even if in a case like this the root of the problem isn't config-specific, very often it's something like an index access in a generic that doesn't affect the computed type at all and could have been safely ignored by the end-user. The resulting issue caused for both the user and maintainer can be difficult to track down and address, whereas I'd question how often the average TS user with skipLibCheck defaulted is being alerted to useful problems (which I guess they're supposed to pass on to maintainers?).

The only path I can see to this being a reliable source of errors is if TS could load multiple TSConfigs simultaneously and apply per-directory settings so that dependencies had control over the rules that applied to their own types, but I assume that would be a massive undertaking. Barring that or a default change, maybe some gold standard for "check your types against this config, if they work here then they should work with any config" could be adopted by libraries hoping to avoid this (not sure whether this possible or some settings have mutually exclusive results).

EDIT: Just found #49886, so maybe these is hope for the required subset to be included as part of the compilation if the performance issues can be mitigated. Though I'd still question whether skipLibChecks would be a net positive, I'd feel much better about it if I could include our config options in our .d.ts outputπŸ‘

@Andarist
Copy link
Contributor

It seems that one of my open PRs already fixes this: #54759 . I just pushed out this as an extra test case there

@RyanCavanaugh RyanCavanaugh added the Rescheduled This issue was previously scheduled to an earlier milestone label Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue Rescheduled This issue was previously scheduled to an earlier milestone
Projects
None yet
5 participants