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

Regression in advanced nested types in TS 3.2 #29168

Closed
cyrilletuzi opened this issue Dec 27, 2018 · 8 comments
Closed

Regression in advanced nested types in TS 3.2 #29168

cyrilletuzi opened this issue Dec 27, 2018 · 8 comments
Labels
Bug A bug in TypeScript Domain: Contextual Types The issue relates to contextual types

Comments

@cyrilletuzi
Copy link

cyrilletuzi commented Dec 27, 2018

TypeScript Version: 3.2.2 & 3.3.0-dev.20181222
Problem started in TS 3.2 and is still there in next version.

Search Terms:
nested

Code

interface TestObject {
  type?: 'object';
  items: {
    [k: string]: TestGeneric;
  };
}

interface TestString {
  type: 'string';
}

type TestGeneric = (TestString | TestObject) & { [k: string]: any; };

const test: TestGeneric = {
  items: {
    hello: { type: 'string' },
    world: {
      items: {
        nested: { type: 'string' }
      }
    }
  }
};

Expected behavior:

Worked in TS 3.1.

Actual behavior:

Error since TS 3.2. The exact error varies from one variable to another, but it seems that when there is nesting, TS infers the first property correctly (let's say it's TestString), but fails on the next ones because it checks with the same inference as the first property (TestString), while it could be another one (TestObject).

It works if TestGeneric is just:

type TestGeneric = (TestString | TestObject);

without { [k: string]: any; }.

Use case:

cyrilletuzi/angular-async-local-storage#64

JSON schemas are structured this way. You can see an example in this lib, which uses JSON schemas for validation:

  • properties of a JSON schema can be another JSON Schema, so it can be nested ;
  • { [k: string]: any; } is added because the lib doesn't support all the features of the JSON schema standard, and I want the lib to allow standard JSON schemas without breaking.
@jack-williams
Copy link
Collaborator

jack-williams commented Dec 27, 2018

I'm afraid that I don't know the exact feature that caused the regression, but I think the problem is that the string literal inference is being lost, probably due to the intersection with { [k: string]: any; }. For example, this works:

const test: TestGeneric = {
    items: {
        hello: { type: 'string' as 'string' },
        world: {
            items: {
                nested: { type: 'string' as 'string' }
            }
        }
    }
};

Think I tracked the new behaviour to this PR #27849.
PR up at #29174.

@cyrilletuzi
Copy link
Author

cyrilletuzi commented Dec 27, 2018

Yes it works if nested types are casted:

const test: TestGeneric = {
  items: {
    hello: { type: 'string' },
    world: {
      items: {
        nested: { type: 'string' } as TestString
      }
    }
  }
};

@cyrilletuzi
Copy link
Author

@weswigham As this is a regression, shouldn't this be planned as a patch update to TS 3.2?

@weswigham
Copy link
Member

AFAIK we don't have another patch release planned before 3.3, but I could be wrong.

@cyrilletuzi
Copy link
Author

cyrilletuzi commented Jan 2, 2019

I'm surprised by the releases management of TypeScript. If there is a regression, shouldn't a patch be planned even it was not planned before?

Because what is told here, in the real world, means that: people using Angular and the lib mentioned (which is a quite used one) will update to Angular 7.2 soon, which will also update to TypeScript 3.2 through the Angular CLI, and then the compilation of their apps will break... And they won't be able to update to TypeScript 3.3 as it will need some time for Angular to support it (and it's not even released...).

@weswigham
Copy link
Member

I mean, if we end up putting a patch release we'd consider having the fix in it should it be ready, but we don't generally cut patch releases for old versions of TS once the new version is out (and 3.3 is probably sometime in the next two or three weeks). The effort required to port a change to all the possible versions (and then test it) makes it pretty infeasible for us to keep maintaining older minor releases.

@DanielRosenwasser would know more about our release plans.

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Jan 10, 2019

We usually decide depending on the proximity of the next release, the effort, and the impact of the issue. I've been pretty liberal on porting high-impact fixes over.

@cyrilletuzi
Copy link
Author

Closing, as it seems to be fixed by #29478.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: Contextual Types The issue relates to contextual types
Projects
None yet
Development

No branches or pull requests

5 participants