Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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

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

Successfully merging this pull request may close these issues.

2 participants