Skip to content

update signal test to use vitest expectTypeOf#5127

Open
santoshyadavdev wants to merge 4 commits intongrx:mainfrom
santoshyadavdev:migrate-to-expectTypeOf
Open

update signal test to use vitest expectTypeOf#5127
santoshyadavdev wants to merge 4 commits intongrx:mainfrom
santoshyadavdev:migrate-to-expectTypeOf

Conversation

@santoshyadavdev
Copy link
Contributor

@santoshyadavdev santoshyadavdev commented Mar 25, 2026

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?

uses expectTypeOf for type testing, this closed #4935 partially

Does this PR introduce a breaking change?

[ ] Yes
[ ] No

Other information

@netlify
Copy link

netlify bot commented Mar 25, 2026

Deploy Preview for ngrx-io ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit dce9057
🔍 Latest deploy log https://app.netlify.com/projects/ngrx-io/deploys/69c44d819ee33f000871e9f1
😎 Deploy Preview https://deploy-preview-5127--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.

/Property 'numbers' is missing in type '{ count: number; foo: string; }'/
);
const state = signalState({ count: 1, foo: 'bar' });
void (() => {
Copy link
Member

Choose a reason for hiding this comment

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

Why is this in a callback function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ohh no leftover, I will replace it, I thought I already did

Copy link
Contributor Author

@santoshyadavdev santoshyadavdev Mar 25, 2026

Choose a reason for hiding this comment

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

@timdeschryver i think we need arrow function, earlier these calls were string so no actual call was happening, but If i dont wrap it around arrow function, ts will execute addNumber and it will always throw exception as its not an array, let me know if there is some clean way to do this

Copy link
Contributor

@rainerhahnekamp rainerhahnekamp left a comment

Choose a reason for hiding this comment

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

@santoshyadavdev. Looks reasonable to me at first sight. Still have to check it thoroughly, but it seems we will settle down with a combination of @ts-expect-error and Vitest type assertions?

@santoshyadavdev
Copy link
Contributor Author

@santoshyadavdev. Looks reasonable to me at first sight. Still have to check it thoroughly, but it seems we will settle down with a combination of @ts-expect-error and Vitest type assertions?

Yes you are right, I will fix some pending code and linter soon

@mrazauskas
Copy link

Do I get it right that pnpm exec nx test signals should run the updated tests? Hm.. I tried adding obviously failing line (like const a: number = "abc") to modules/signals/spec/types/signal-state.types.spec.ts. Running the command reports all tests passing. Or did I misunderstood something?

@rainerhahnekamp Would be be still interested to check out TSTyche? (Based on #4935 (comment)). I could put together a similar PR. One of the advantages, there would be no need for @ts-expect-error directives, that cast too wide a net (in type testing context). Because any errors in the following line are suppressed.

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.

Replace ts-snippet

4 participants