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

fix(jest): fix for incorrect typing of SpyObject for Jest #96

Merged
merged 1 commit into from Mar 30, 2019
Merged

fix(jest): fix for incorrect typing of SpyObject for Jest #96

merged 1 commit into from Mar 30, 2019

Conversation

dirkluijk
Copy link
Collaborator

Just upgraded to Jest 24, and got compiler errors. I think the reason this bug was nog blocking before, is that @types/jest@24.0.0 is more strict with generics.

The issue
Given an interface:

interface Person {
  name: string;
  saySomething: () => string;
}

when you define SpyObject<Person> as { [P in keyof Person]: Person[P] & jest.Mock<Person>, then personSpyObject.saySomething would be a () => string & jest.Mock<Person>. That is wrong, because it should be a jest.Mock<string>.

To get the return type of Person[P] I use ReturnType combined with conditional types from Typescript 2.8. Awesome stuff by the way.

@dirkluijk
Copy link
Collaborator Author

Build is now stable.

@NetanelBasal
Copy link
Member

It might be a breaking change because maybe there are people that from some reason are still in < 2.8.

@dirkluijk
Copy link
Collaborator Author

Strictly speaking yes.

I checked it on our project, but this change seems to be perfectly backwards compatible with Jest 23.x.

The reason is that it was already wrong since the beginning of the Jest support in Spectator, but Jest 24 has a lot of TypeScript improvements and on Jest 23.x one would not get any errors from it.

@dirkluijk
Copy link
Collaborator Author

dirkluijk commented Mar 21, 2019

Oh, sorry. You meant Typescript < 2.8.
Yes, totally right. Didn't think of that.

Angular 6 depends on TypeScript 2.7
but Angular 7 depends on TypeScript 3.1
and Angular 8 depends on TypeScript 3.3.

Can we drop support for Angular 6 and introduce a new (major?) version of Spectator?

Alternative would be: just Mock<any>. But then you have no type checks on mocked functions.

@NetanelBasal
Copy link
Member

Yes, and you could also upgrade to Jest 24?

@dirkluijk
Copy link
Collaborator Author

Yes, but this fix is required for Jest 24. ;) Otherwise you get compile errors.

@NetanelBasal
Copy link
Member

Sure

@dirkluijk
Copy link
Collaborator Author

@NetanelBasal what fix would you suggest for now? Mock<any> or the breaking TS 2.8 change? 😁

@NetanelBasal
Copy link
Member

I'm suggesting to release a new major version and update both Jest and Typescript.

@dirkluijk
Copy link
Collaborator Author

Just simplified it a bit and added Typescript 2.8 as peer dependency.

@NetanelBasal
Copy link
Member

Does this require a major release?

@dirkluijk
Copy link
Collaborator Author

I ‘m not sure. Spectator API itself hasn’t changed, but its peer dependencies have.

Strictly speaking, it should. I know Angular does TS upgrades for major versions as well.

See also semver/semver#148.

It’s up to you to decide. ;)

@NetanelBasal
Copy link
Member

I'm not sure if someone is still using Angular 6 🤔. I don't think it's worth a new major version.

@NetanelBasal NetanelBasal merged commit 134ab71 into ngneat:master Mar 30, 2019
@NetanelBasal
Copy link
Member

3.6.0

@dirkluijk dirkluijk deleted the fix/jest-typing-fix branch March 30, 2019 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants