test(entity): refactor to Vitest type tests#5146
Conversation
✅ Deploy Preview for ngrx-io ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
@copilot resolve the merge conflicts in this pull request 👀 |
db95327 to
3d1d0e4
Compare
rainerhahnekamp
left a comment
There was a problem hiding this comment.
@timdeschryver as mentioned in the commit, I think it would be good, if do the split of type-only and "hybrid type tests" (those which have @ts-expect-error)
| it('sets the id type to string when the entity has a string id', () => { | ||
| const adapter = createEntityAdapter<EntityWithStringId>(); | ||
|
|
||
| expectTypeOf(adapter).toEqualTypeOf< |
There was a problem hiding this comment.
@tim would it make sense to move the type-only tests into a test-d.ts file? I think it would be good if we would have some those files which can serve as a template for future tests.
There was a problem hiding this comment.
Yes, you're right. I resolved the merge conflicts, but didn't pay attention to it.
This will also help for cases as mentioned in #5127 (comment)
| EntitySelectors<unknown, Record<string, any>> | ||
| >; | ||
| `).toInfer('result', 'true'); | ||
| expectTypeOf< |
There was a problem hiding this comment.
I would see that file as test-d.ts test. The advantage would be that they would never be executed - only compilted.
Move pure type-only tests (using expectTypeOf) to .test-d.ts files so they are only compiled and never executed. Keep hybrid tests that use @ts-expect-error in .types.spec.ts files. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Closes #
What is the new behavior?
Does this PR introduce a breaking change?
Other information