Skip to content

test(entity): refactor to Vitest type tests#5146

Open
timdeschryver wants to merge 4 commits into
mainfrom
vitest/entity
Open

test(entity): refactor to Vitest type tests#5146
timdeschryver wants to merge 4 commits into
mainfrom
vitest/entity

Conversation

@timdeschryver

Copy link
Copy Markdown
Member

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[x] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Closes #

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[ ] No

Other information

@netlify

netlify Bot commented Apr 16, 2026

Copy link
Copy Markdown

Deploy Preview for ngrx-io ready!

Name Link
🔨 Latest commit db95327
🔍 Latest deploy log https://app.netlify.com/projects/ngrx-io/deploys/69e137667f8ce5000897810e
😎 Deploy Preview https://deploy-preview-5146--ngrx-io.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@timdeschryver

Copy link
Copy Markdown
Member Author

@copilot resolve the merge conflicts in this pull request 👀

@rainerhahnekamp rainerhahnekamp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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<

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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<

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
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.

3 participants