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

Remove prdicate typing for query filters #8958

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Nick-Lucas
Copy link
Contributor

No description provided.

Copy link

nx-cloud bot commented Apr 5, 2025

View your CI Pipeline Execution ↗ for commit 65a9840.

Command Status Duration Result
nx affected --targets=test:sherif,test:knip,tes... ❌ Failed 2m 31s View ↗
nx run-many --target=build --exclude=examples/*... ❌ Failed 6s View ↗

☁️ Nx Cloud last updated this comment at 2025-04-05 09:44:14 UTC

@Nick-Lucas
Copy link
Contributor Author

So @TkDodo I'm in two minds if we should go as far as stripping out the generics entirely. It's more of a breaking change to remove them than to leave them unused, especially since the 4th type param (query key) does get used currently.

What do you think?

@TkDodo
Copy link
Collaborator

TkDodo commented Apr 5, 2025

TypeScript errors on unused type parameters:

src/utils.ts(19,3): error TS6133: 'TError' is declared but its value is never read.
src/utils.ts(20,3): error TS6133: 'TData' is declared but its value is never read.

so what option do we have other than deleting them?

@Nick-Lucas
Copy link
Contributor Author

Should be possible to ignore the warnings I think? So it's more a question of where your mind is at as a maintainer

I didn't do anything today beyond start this draft, there will be tests failing as well that I can sort out either way

@TkDodo
Copy link
Collaborator

TkDodo commented Apr 5, 2025

Should be possible to ignore the warnings I think?

this isn’t just an eslint warning - it’s a TS error that you would need to silence with ts-ignore, which is not something I’d recommend because if the comments get stripped, the error might resurface

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

Successfully merging this pull request may close these issues.

2 participants