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

Typescript compiler flags error in generated forgotPassword.test.ts file #188

Closed
roshan-sama opened this issue Mar 8, 2021 · 9 comments · Fixed by blitz-js/blitz#2110
Closed

Comments

@roshan-sama
Copy link
Contributor

What is the problem?

In line 12 of \app\auth\mutations\forgotPassword.test.ts, ...jest.requireActual("blitz")!, causes tsc to throw this error: Spread types may only be created from object types.ts(2698)

Seems like the issue is that spread types are not supported on generics (and I'm assuming that the jest.requireActual("blitz") call is returning an object, but the signature denotes that it's "unkown", which is what is causing the issue?
function jest.requireActual<unknown>(moduleName: string): unknown

There's an open issue at microsoft/TypeScript#10727, so this might not be a blitz related issue, but it may make sense to implement a workaround in the meanwhile

Paste all your error logs here:

tsc
app/auth/mutations/forgotPassword.test.ts:12:3 - error TS2698: Spread types may only be created from object types.

12   ...jest.requireActual("blitz")!,
     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Found 1 error

Paste all relevant code snippets here:

jest.mock("blitz", () => ({
  ...jest.requireActual("blitz")!,      <--------- This line
  generateToken: () => generatedToken,
}))

What are detailed steps to reproduce this?

  1. Download latest blitz, create a new app and run the tsc command

Run blitz -v and paste the output here:

Windows 10 | win32-x64 | Node: v15.8.0

blitz: 0.31.1 (global)
blitz: 0.31.1 (local)

  Package manager: yarn
  System:
    OS: Windows 10 10.0.18363
    CPU: (16) x64 AMD Ryzen 7 2700 Eight-Core Processor
    Memory: 5.49 GB / 15.93 GB
  Binaries:
    Node: 15.8.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.10 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 7.6.0 - C:\Program Files\nodejs\npm.CMD
    Watchman: Not Found
  npmPackages:
    @prisma/client: ~2.17 => 2.17.0
    blitz: 0.31.1 => 0.31.1
    prisma: ~2.17 => 2.17.0
    react: 0.0.0-experimental-3310209d0 => 0.0.0-experimental-3310209d0
    react-dom: 0.0.0-experimental-3310209d0 => 0.0.0-experimental-3310209d0
    typescript: ~4.1 => 4.1.5
@roshan-sama
Copy link
Contributor Author

I'll have to note that I'm running tsc manually because I broke husky somehow, so if this was something fixed via code in a hook via Husky, please feel free to close!

@roshan-sama
Copy link
Contributor Author

roshan-sama commented Mar 8, 2021

Update 2:

Doing ...jest.requireActual<object>("blitz")! fixes the issue. Should we do this then?

@flybayer
Copy link
Member

flybayer commented Mar 8, 2021

Thanks! I think I vote for jest.requireActual("blitz")! because <object> isn't the correct type. So ! expresses the intent better here, where the intent is just to make typescript stop complaining :)

@roshan-sama
Copy link
Contributor Author

Ah, I was going to help do this change, but tsc complains that a comma was expected if I remove the spread operator :((

app/auth/mutations/forgotPassword.test.ts:12:7 - error TS1005: ',' expected.

12   jest.requireActual("blitz")!,
         ~


Found 1 error.

I tried a few permutations with the ! at different spots with no luck, I don't think I have enough TS expertise for this haha. I'll defer to you or someone else for the fix.

@flybayer
Copy link
Member

flybayer commented Mar 9, 2021

@Roesh did ...jest.requireActual<object>("blitz")! not work?

@roshan-sama
Copy link
Contributor Author

@flybayer that did work, and I have it set to that in my personal project, but I assumed we wanted to not have the generic and use jest.requireActual("blitz")!?

I might have misunderstood that comment then. Shall I modify ...jest.requireActual("blitz")! to ...jest.requireActual<object>("blitz")! and submit a PR with that change then?

@flybayer
Copy link
Member

@Roesh I'm still a bit fuzzy. I think this is what you are saying:

  • Works: ...jest.requireActual("blitz")!
  • Works: ...jest.requireActual<object>("blitz")!

Since both work, then let's go the more minimal approach and use ...jest.requireActual("blitz")!

@roshan-sama
Copy link
Contributor Author

@flybayer Ah, I didn't mean that, rather

...jest.requireActual("blitz")! (what it is originally) causes typescript to complain that error TS2698: Spread types may only be created from object types.

However, ...jest.requireActual<object>("blitz")! works and typescript doesn't throw that error.

Does tsc not throw the same error for you at ...jest.requireActual("blitz")!? My typescript version is Version 4.2.3

@flybayer
Copy link
Member

@Roesh ah got it! Ok then yes, make a PR with <object>. No, currently on TS 4.1 which is probably why

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants