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

Surprising circularity error in 3.9 RC #38279

Closed
amcasey opened this issue Apr 30, 2020 · 20 comments · Fixed by #38653
Closed

Surprising circularity error in 3.9 RC #38279

amcasey opened this issue Apr 30, 2020 · 20 comments · Fixed by #38653
Assignees
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue

Comments

@amcasey
Copy link
Member

amcasey commented Apr 30, 2020

I haven't had a chance to reduce the repro, but people seem eager to get started. I'll try to update with better steps. For now:

  1. git clone --depth=1 https://github.com/davidkpiano/xstate
  2. yarn install --ignore-scripts
  3. tsc -b -f packages/xstate-vue
  4. Fix packages/core/src/actions.ts:108:9 - error TS2783: 'type' is specified more than once, so this usage will be overwritten.
  5. tsc -b -f packages/xstate-vue

Expected: no errors, as in 3.8.3
Actual:

packages/xstate-vue/src/xstate-machine.ts:16:14 - error TS2615: Type of property 'machine' circularly references itself in mapped type 'RecordPropsDefinition<{ machine: any; options: any; }>'.

 16     machine: {
                 ~
 17       type: Object,
    ~~~~~~~~~~~~~~~~~~~
... 
 19       default: () => ({})
    ~~~~~~~~~~~~~~~~~~~~~~~~~
 20     },
    ~~~~~

packages/xstate-vue/src/xstate-machine.ts:23:23 - error TS2615: Type of property 'options' circularly references itself in mapped type 'RecordPropsDefinition<{ machine: any; options: any; }>'.

23       default: () => ({})
                         ~~

TypeScript Version: 3.9.1-rc

@amcasey
Copy link
Member Author

amcasey commented Apr 30, 2020

FYI @DanielRosenwasser @ahejlsberg

@amcasey
Copy link
Member Author

amcasey commented Apr 30, 2020

I introduced const dummy = {} and replaced both lambdas with () => dummy and the error went away.

@DanielRosenwasser DanielRosenwasser added the Needs Investigation This issue needs a team member to investigate its status. label Apr 30, 2020
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 3.9.2 milestone Apr 30, 2020
@amcasey
Copy link
Member Author

amcasey commented Apr 30, 2020

@RyanCavanaugh We didn't catch this in our last pass because it was downstream from a boring break.

@ahejlsberg
Copy link
Member

This looks to be a case of an object literal being contextually typed by itself. I'm pretty sure the reason we didn't report it previously is that during mapped type member resolution we'd temporarily set the result to an empty array, eagerly compute all the member types, and then overwrite that empty array with the final result. We now properly defer resolution of mapped type member types which enables us to catch circularities and report them. And that's what's happening here.

@ahejlsberg
Copy link
Member

The quick fix here is to add a {} type annotation: default: (): {} => ({}).

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Apr 30, 2020

Or any explicit type - probably Record<string, any>. But I think that my fear is whether this causes a lot of issues for Vue projects.

@DanielRosenwasser
Copy link
Member

@ktsn @HerringtonDarkholme @Kingwl @jackkoppa @blake-newman have any of you been able to give TypeScript 3.9 a try with Vue?

@weswigham
Copy link
Member

I think "object ends up contextually typing itself" is actually a pretty common scenario in vue. 😦 I know I've debugged many an issue in the past with that exact premise, and I've usually made the fix something along the lines of adjusting contextual typing or inference so the object no longer depended upon itself.

@HerringtonDarkholme
Copy link
Contributor

HerringtonDarkholme commented May 5, 2020

It seems to be an issue about mapping type and function overload.

I can reproduce the issue in a minimal fashion:

type Func<T> = () => T

type Mapped<T> = {
  [K in keyof T]: Func<T[K]> // error
  // [K in keyof T]: () => T[K] // this compiles
}

declare function reproduce(options: number): void // removing this line also compiles
declare function reproduce<T>(options: Mapped<T>): T


reproduce({
  name:   () => { return 123 } // ERROR
})

reproduce({
  name() { return 123 } // ok
})

reproduce({
  name: function () { return 123 } // ok
})

Curiously, without the type alias Func<T>, the code above compiles. Also, removing the overload also makes the code compile.

Even weirder, changing arrow function to method or plain function also makes it compile.

Playground link for nightly build

https://www.typescriptlang.org/play/index.html?noImplicitThis=false&allowUnreachableCode=true&allowUnusedLabels=true&downlevelIteration=true&noEmitHelpers=true&esModuleInterop=false&declaration=false&target=99&ts=4.0.0-dev.20200504&ssl=22&ssc=3&pln=1&pc=1#code/C4TwDgpgBAYgrgOwMYB4AqA+KBeKAKAShyzQFgAoC0SKAWQEMxIATdLXAbwqigG0BpKAEsEUANYQQAewBmUNAF0AXLESo0AhRm5QA9Lr6CR4ybPnL8RbCU16DwABZCAzlCRSAtmCEAbCM4oAXwoKZggkH3oAJ2gZNWAhKVEYsCipZjgkCDwpMASk5xUEOA8AIwgogiKS8qi7KBiPKQA3EQBzKEcXKB8RaHofZyk3T28-APIwiOjY+MTkiFT0zIg2HLz5wrpGFjYq+RDKchS0jKy8LnIeBHoPCBUeQmIoDgaIYDgo0QBGACYAZigwXIgQIhxOy3Ol2ut2yRFeMQ+Xygf0BwNB4MWpxWFx0NzuKjiyHyoieCPenx+AKBQQIQA

@samchon
Copy link

samchon commented May 15, 2020

samchon/tgrid#34

src/components/Driver.ts:41:17 - error TS2456: Type alias 'Promisive' circularly references itself.

Well, this error can be a same one? I've found similar error message when using recursive generic type. The error only occured in the v3.9.2 version. No error in the old v3.8.3 version.

@jackkoppa
Copy link

jackkoppa commented May 16, 2020

Thanks for checking, @DanielRosenwasser - just tried out 3.9.2 in our two largest TS + Vue repos, and yup, I'm seeing this type of error (28 instances across the 2 repos). Primarily, for all our props that define defaults, returning arrays. Seems to match the description of what's going on

Screen Shot 2020-05-16 at 5 42 59 PM

All in all, it wouldn't be a huge deal-breaker for TS + Vue devs, to have to add return type annotations to those methods, though a bit of a pain. I primarily worry for people who run into the problem as they upgrade, given that Vue 2 is inherently a bit fragile in its type definitions (with all the circularity involved); since the error message is about the way RecordPropsDefinition is circular, I don't know if most users will be able to quickly find their way to this issue & know to annotate the default function.

@jackkoppa
Copy link

@ahejlsberg ahejlsberg added Bug A bug in TypeScript and removed Needs Investigation This issue needs a team member to investigate its status. labels May 19, 2020
@ahejlsberg
Copy link
Member

Since this is a regression I'm going to label it a bug. As I said above, the issue here is an object literal that is contextually typed by itself. I think we can fix it by not producing a contextual type in cases where resolution of a contextual mapped type property would cause a circularity error.

@DavidRouyer
Copy link

Please reopen, the issue still persists on TypeScript 3.9.3. See @jackkoppa repo and bump the TypeScript version to 3.9.3. Circular references are still there so TypeScript 3.9 can't be currently used on Vue projects.

@RyanCavanaugh
Copy link
Member

We'll include this in our May Servicing release

@mpawelski
Copy link

Is this included in 3.9.5?
the milestone and release notes suggests that it should, but I still get this errors in 3.9.5.

how to reproduce:

  1. git clone https://github.com/mpawelski/vuejs_ts_3_9_issue.git
  2. cd vuejs_ts_3_9_issue
  3. yarn
  4. npx typescript@3.9.5 doesn't work
  5. npx typescript@3.8.3 works

@weswigham
Copy link
Member

The change never got accepted into the 3.9 branch: #38687

@jackkoppa
Copy link

Yup, can confirm that we still see the same issue in 3.9.5. Let us know if we can help with any additional testing

@weswigham
Copy link
Member

@RyanCavanaugh is this going to get ported/accepted into a 3.9 release at some point? Because right now the milestone is at least misleading, since it didn't go out in the last one.

@tony-scio
Copy link

tony-scio commented Jul 2, 2020

Did this go into 3.9.6? I'm still blocked on upgrading to 3.9 due to unexpected circularity errors in 3.9.6 (repro in piotrwitek/utility-types#144). I have no idea if the bug I'm tripping over is actually a TS bug or a utility-types bug.

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
Projects
None yet
Development

Successfully merging a pull request may close this issue.