GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Improve soundness of indexed access types by ahejlsberg · Pull Request #30769 · microsoft/TypeScript

Conversation

@ahejlsberg

@ahejlsberg Anders Hejlsberg (ahejlsberg) commented Apr 5, 2019

Copy link
Copy Markdown
Member

With this PR we improve soundness of indexed access types in a number of ways:

  • When an indexed access T[K] occurs on the source side of a type relationship, it resolves to a union type of the properties selected by T[K], but when it occurs on the target side of a type relationship, it now resolves to an intersection type of the properties selected by T[K]. Previously, the target side would resolve to a union type as well, which is unsound.
  • Given a type variable T with a constraint C, when an indexed access T[K] occurs on the target side of a type relationship, index signatures in C are now ignored. This is because a type argument for T isn't actually required to have an index signature, it is just required to have properties with matching types.
  • A type { [key: string]: number } is no longer related to a mapped type { [P in K]: number }, where K is a type variable. This is consistent with a string index signature in the source not matching actual properties in the target.
  • Constraints of indexed access types are now more thoroughly explored. For example, given type variables T and K extends 'a' | 'b', the types { a: T, b: T }[K] and T are now considered related where previously they weren't.

Some examples:

function f1(obj: { a: number, b: string }, key: 'a' | 'b') {
    obj[key] = 1;    // Error
    obj[key] = 'x';  // Error
}

function f2(obj: { a: number, b: 0 | 1 }, key: 'a' | 'b') {
    obj[key] = 1;
    obj[key] = 2;  // Error
}

function f3<T extends { [key: string]: any }>(obj: T) {
    let foo = obj['foo'];
    let bar = obj['bar'];
    obj['foo'] = 123;  // Error
    obj['bar'] = 'x';  // Error
}

function f4<K extends string>(a: { [P in K]: number }, b: { [key: string]: number }) {
    a = b;  // Error
    b = a;
}

Previously, none of the above errors were reported.

Fixes #27895.
Fixes #30603.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

alexrock, j-oliveras, karol-majewski, Bnaya, aboyton, bjornua, Nathan-Fenner, Igorbek, trotyl, ahagelstein, and 18 more reacted with thumbs up emoji TidyIQ, fengcen, archfz, timonson, dmaklygin, DzmVasileusky, justjake, swittk, cdpark0530, and sliminality reacted with thumbs down emoji dragomirtitian, j-oliveras, Bnaya, Igorbek, trotyl, bjornua, jcalz, unao, and millsp reacted with hooray emoji archfz reacted with confused emoji trotyl, jcalz, unao, and millsp reacted with heart emoji SlurpTheo reacted with eyes emoji
All reactions
  • 👍 28 reactions
  • 👎 10 reactions
  • 🎉 9 reactions
  • 😕 1 reaction
  • ❤️ 4 reactions
  • 👀 1 reaction
Anders Hejlsberg (ahejlsberg) added 11 commits March 29, 2019 13:00
@ahejlsberg
Improve soundness of indexed access type relations
fc95dc4
@ahejlsberg
getIndexedAccessType for assignment target uses intersections
5de3ee8
@ahejlsberg
Add getIndexedAccessOrUndefined function
95c8a92
@ahejlsberg
Accept new baselines
3e09d29
@ahejlsberg
More exploration of indexed access type constraints in type relations
d61a693
@ahejlsberg
Minor fixes
1648d0c
@ahejlsberg
Accept new baselines
e79e0bd
@ahejlsberg
T[K] ignores index signatures in T's constraint
d69aeb4
@ahejlsberg
Accept new baselines
f5f1f30
@ahejlsberg
Add tests
739acee
@ahejlsberg
Accept new baselines
e01fa85
@jack-williams

Jack Williams (jack-williams) commented Apr 5, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator

Has the behaviour that was added in #27490 been taken out? The following function did error, but not under the new rules.

function fun<U extends { a: T }, T, K extends 'a'>(u: U, k: K, shambles: T): U[K] {
  u[k] = shambles; // was error
  return u[k];
}

const result: number = fun<{ a: number }, unknown, 'a'>({ a: 42 }, 'a', 'a string');

I think the reason that the test-cases from #27470 didn't regress is because the restrictive instantiation has been subsequently added to conditional types, which catches the error too.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ahejlsberg
Merge branch 'master' into saferIndexedAccessTypes
2945802
@ahejlsberg

Anders Hejlsberg (ahejlsberg) commented Apr 6, 2019

Copy link
Copy Markdown
Member Author

Has the behaviour that was added in #27490 been taken out?

Yes, but that behavior wasn't right. For an indexed access T[K] we flat out didn't explore any constraints on the target side when both T and K were generic, which is quite inconsistent. For example, if you change the assignment in your example from u[k] = shambles to u['a'] = shambles there is no error before or now.

The real (and orthogonal) issue in your example is that we treat an assignment to T[K] as an assignment to C[K], where C is the constraint of T. I discuss the reasons in my comment here. The unsoundness caused by that rule continues to exist (and would be quite a bit harder to remove).

What has changed with this PR is that for a T[K] on the target side of a relation we now ignore index signatures in the constraint of T. That's definitely the right thing to do (because a type argument for T isn't actually required to have an index signature, it is merely required to have properties with matching types) and that's what really fixes the issue in #27470.

It's all rather involved!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ahejlsberg
Accept new baselines
9832a83
@ahejlsberg

Anders Hejlsberg (ahejlsberg) commented Apr 6, 2019

Copy link
Copy Markdown
Member Author

Wesley Wigham (@weswigham) Can you take a look at why this is failing on PragmaPsuedoMap?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@weswigham

Wesley Wigham (weswigham) commented Apr 6, 2019

Copy link
Copy Markdown
Member

Looks like we're pulling out some unexpected index signatures from the target or something? The bug should repro in the test harness, will just need to copy the relevant types into a test file.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ahejlsberg

Anders Hejlsberg (ahejlsberg) commented Apr 6, 2019

Copy link
Copy Markdown
Member Author

Wesley Wigham (@weswigham) It's actually identifying an unsafe coercion. It used to be that PragmaPseudoMap["reference"] was (unsoundly) assignable to PragmaPseudoMap[K], where K extends keyof PragmaPseudoMap, which, in combination with parameter bi-variance (because the compiler isn't yet using --strictFunctionTypes), caused the explicit type annotation on (arg: PragmaPseudoMap["reference"]) => ... to be accepted. That's no longer the case, and that's a good thing.

The easy fix is to explicitly cast the array parameter to forEach to the appropriate type.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ahejlsberg
Fix unsafe parameter coercions related to PragmaPseudoMap
e1fd5e5
@ahejlsberg

Anders Hejlsberg (ahejlsberg) commented Apr 6, 2019

Copy link
Copy Markdown
Member Author

TypeScript Bot (@typescript-bot) test this

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@typescript-bot

TypeScript Bot (typescript-bot) commented Apr 6, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Heya Anders Hejlsberg (@ahejlsberg), I've started to run the extended test suite on this PR at e1fd5e5. You can monitor the build here. It should now contribute to this PR's status checks.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ahejlsberg

Anders Hejlsberg (ahejlsberg) commented Apr 6, 2019

Copy link
Copy Markdown
Member Author

TypeScript Bot (@typescript-bot) run dt

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@typescript-bot

TypeScript Bot (typescript-bot) commented Apr 6, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Heya Anders Hejlsberg (@ahejlsberg), I've started to run the Definitely Typed test suite on this PR at e1fd5e5. You can monitor the build here. It should now contribute to this PR's status checks.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@jack-williams

Jack Williams (jack-williams) commented Apr 6, 2019

Copy link
Copy Markdown
Collaborator

Thanks for the detailed write-up Anders Hejlsberg (@ahejlsberg).

The real (and orthogonal) issue in your example is that we treat an assignment to T[K] as an assignment to C[K], where C is the constraint of T. I discuss the reasons in my comment here. The unsoundness caused by that rule continues to exist (and would be quite a bit harder to remove).

True, though did you not add behaviour to prevent pulling down C when T and K were both generic variables which is no longer in this PR? I understand that the original issue that prompted the fix is now correctly handled for other (more sensible) reasons, but discarding a sound rule seems a shame! Though, the rule is abit ad-hoc!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Anders Hejlsberg (ahejlsberg) added 4 commits April 8, 2019 06:36
@ahejlsberg
Simplify obtaining base constraint of T[K] for writing
b49a343
@ahejlsberg
Accept new baselines
71f3e1f
@ahejlsberg
Optimize T[K] where T has string index signature and no other members
eb1e7ec
@ahejlsberg
Accept new baselines
6851268
@ahejlsberg

Anders Hejlsberg (ahejlsberg) commented Apr 8, 2019

Copy link
Copy Markdown
Member Author

With latest commits there are three RWC projects with new errors. The errors in two of the projects are minor and identify unsoundness that we previously didn't catch. The last project, fp-ts, which already has massive inverted pyramids of error messages, now looks even more daunting. It makes heavy use of indexed access types and when they occur in target positions we create intersection types instead of union types. There are several new errors that appear to be correct with the stricter checking, but it's hard to tell.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ahejlsberg

Anders Hejlsberg (ahejlsberg) commented Apr 8, 2019

Copy link
Copy Markdown
Member Author

TypeScript Bot (@typescript-bot) test this

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@fatcerberus Bruce Pascoe (fatcerberus) mentioned this pull request Aug 12, 2021
Narrow indexing variable when type-testing indexed access expressions #45421
Open
@jcalz Joe Calzaretta (jcalz) mentioned this pull request Oct 11, 2021
typeof item in Array incrorrect #46304
Closed
@fatcerberus Bruce Pascoe (fatcerberus) mentioned this pull request Jan 3, 2022
Proxy property in setter has never type #47295
Closed
@jcalz Joe Calzaretta (jcalz) mentioned this pull request Jan 9, 2022
Two level deep indexed access: read works, write fails #47352
Closed
@quixot1c

quixot1c commented Jan 11, 2022

Copy link
Copy Markdown
  • Given a type variable T with a constraint C, when an indexed access T[K] occurs on the target side of a type relationship, index signatures in C are now ignored. This is because a type argument for T isn't actually required to have an index signature, it is just required to have properties with matching types.

Anders Hejlsberg (@ahejlsberg) If "a type argument for T isn't actually required to have an index signature", why is indexed access still allowed on the source side of a type relationship?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ahejlsberg

Anders Hejlsberg (ahejlsberg) commented Jan 11, 2022

Copy link
Copy Markdown
Member Author

If "a type argument for T isn't actually required to have an index signature", why is indexed access still allowed on the source side of a type relationship?

An indexed access T[K] doesn't actually require T to have an index signature. It's more helpful to think of it as a property access. As long as K is known to reference the name of a property in T (i.e. because K is a literal type like "foo"), then T[K] is permitted. However, an indexed access T[string] does require T to have an index signature.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@jcalz Joe Calzaretta (jcalz) mentioned this pull request Jan 16, 2022
Switch inside for loop is converting to intersection #47459
Closed
@fatcerberus Bruce Pascoe (fatcerberus) mentioned this pull request Jan 20, 2022
Partial<T>[K] is not assignable to T[K] | undefined #47523
Closed
@whzx5byb whzx5byb mentioned this pull request Mar 16, 2022
Generic types combined with & instead of | #48287
Closed
@guevara goddog (guevara) mentioned this pull request Mar 26, 2022
Type polymorphic functions in TypeScript - zhenghao guevara/read-it-later#8384
Open
@gabritto Gabriela Araujo Britto (gabritto) mentioned this pull request Apr 6, 2022
Type assignable to index access of constraint of type parameter is incorrectly assignable to the index access itself #46076
Closed
@fatcerberus Bruce Pascoe (fatcerberus) mentioned this pull request Jun 5, 2022
this index with unions outputs intersections? #49393
Closed
@fatcerberus Bruce Pascoe (fatcerberus) mentioned this pull request Aug 2, 2022
Union of a nullable and not nullable setter results wrongly into a nullable setter #50142
Closed
@whzx5byb whzx5byb mentioned this pull request Aug 2, 2022
Compute writeType from set accessors for union and intersection properties #47674
Merged
@jcalz Joe Calzaretta (jcalz) mentioned this pull request Oct 10, 2022
String index into a generic type having a string index signature isn't assignable to indexed access type #51127
Open
@fatcerberus Bruce Pascoe (fatcerberus) mentioned this pull request Oct 25, 2022
Union in a computed property allows any assignment to property value #38663
Open
@jcalz Joe Calzaretta (jcalz) mentioned this pull request Nov 3, 2022
generic bound doesn't work when used in index type with concrete index type #51394
Closed
@fatcerberus Bruce Pascoe (fatcerberus) mentioned this pull request May 9, 2023
Stricter disjunction of interfaces #54051
Open
@darrylnoakes Darryl Noakes (darrylnoakes) mentioned this pull request Jun 1, 2023
Enhance user experience in face of false-positive type errors and type loss #53977
Open
@fatcerberus Bruce Pascoe (fatcerberus) mentioned this pull request Jul 7, 2023
Generic return type (T[K]) is intersection among all properties of T instead of union #54901
Closed
This was referenced Sep 19, 2023
Regression in nightly build related to keyof, type narrowing, and exactOptionalPropertyTypes #55756
Closed
Intersection on indexing a conditional type object #55804
Closed
@Nearoo Silas Gyger (Nearoo) mentioned this pull request Nov 27, 2023
Fix type error in ISpritesheetData pixijs/pixijs#9896
Merged
4 tasks done
@fatcerberus Bruce Pascoe (fatcerberus) mentioned this pull request Dec 29, 2023
setter inferred from union has incorrect variance #56894
Closed
@craigphicks

Craig P Hicks (craigphicks) commented Dec 29, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Could the errors in this test case be overly strict?

function f3<T extends { [key: string]: any }>(obj: T) {
    let foo = obj['foo'];
    let bar = obj['bar'];
    obj['foo'] = 123;  // Error  - (its writable and matches the parameters why error?)
    obj['bar'] = 'x';  // Error - (ditto)
}

This readonly function format exists if errors are desired:

function f5<T extends Readonly<{ [key: string]: any }>>(obj: T) {
    let foo = obj['foo'];
    let bar = obj['bar'];
    obj['foo'] = 123;  // Error
    obj['bar'] = 'x';  // Error
}
declare const obj: {
  foo: boolean;
  bar: boolean;
};
f5(obj);

Shouldn't TypeScript assume the user is reading the function parameters and trust them not to call f3 because of the any type and the fact that it is non-readonly?

Conversely, the user may have a special need for writing a writable any typed function like f3, and now they are not allowed to write it. That could be frustrating.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@jcalz Joe Calzaretta (jcalz) mentioned this pull request Jun 17, 2024
Generic index into two types not seen as assignable, even if the two types are identical for all relevant keys #58905
Closed
@jcalz Joe Calzaretta (jcalz) mentioned this pull request Dec 31, 2024
Type parameter not usable as type argument in identical type; union in constraint becomes intersection #60892
Open
@nmain Nicholai Main (nmain) mentioned this pull request Mar 12, 2025
Unexpected "property is not assignable to type 'undefined'" on dynamic assignment #61409
Closed
@microsoft Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 16, 2025
@typescript-bot TypeScript Bot (typescript-bot) assigned Anders Hejlsberg (ahejlsberg) Oct 16, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@weswigham Wesley Wigham (weswigham) weswigham left review comments
@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) RyanCavanaugh approved these changes
@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) Awaiting requested review from DanielRosenwasser
+1 more reviewer
@jack-williams Jack Williams (jack-williams) jack-williams left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@ahejlsberg Anders Hejlsberg (ahejlsberg)

Labels

Breaking Change Would introduce errors in existing code

Projects

None yet

Milestone

TypeScript 3.5.0

Development

Successfully merging this pull request may close these issues.

Function can return wrong values despite type Change in generic behavior between 3.0.1 and >=3.1.1

Uh oh!

There was an error while loading. Please reload this page.

13 participants

@ahejlsberg @jack-williams @weswigham @typescript-bot @vilicvane @jcalz @fatcerberus @NickHeiner @TidyIQ @k8w @quixot1c @craigphicks @RyanCavanaugh
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.