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

Query syntax problems with TypeBox 0.26 #3182

Open
daffl opened this issue Apr 28, 2023 · 7 comments
Open

Query syntax problems with TypeBox 0.26 #3182

daffl opened this issue Apr 28, 2023 · 7 comments

Comments

@daffl
Copy link
Member

daffl commented Apr 28, 2023

The query syntax helper creates two different types between TypeBox 0.25:

Screenshot_2023-04-20_at_20 46 18

And 0.26:

Screenshot_2023-04-20_at_20 51 28

While the local package tests are passing, in a generated app this means that any operator like

app.service('messages').find({
  query: {
    createdAt: {
      $gte: Date.now() - 24 * 60 * 60 * 1000
    }
  }
})

Will receive a TypeScript error like this:

Property '$something' is incompatible with index signature.
          Type '<type>' is not assignable to type 'never'.

The current solution is to revert to TypeBox 0.25 and probably add a better integration like

Type.QuerySyntax(queryProperties, ['text', 'id'])

To v5.1

@daedalus28
Copy link

Is there any way to help move this along? The typebox dependency is very stale - nearly two years out of date.

I'm happy to attempt a PR for this, but I'm not sure I completely follow the issue being described here. Can you provide a unit test that passes on 0.25 but fails on the latest version?

Alternatively, is there a way we can use the latest typebox directly in our apps while feathers itself still relies on the older version?

@daedalus28
Copy link

I also see #3532 was opened 5 months ago with no engagement on it.

@daffl Any thoughts?

@daffl
Copy link
Member Author

daffl commented Feb 13, 2025

The update is probably still not possible without introducing a breaking change (as the breaking tests in #3532 showed before the logs expired).

I recommend installing your own Typebox dependency in the latest version and using that to explicitly define the schemas you need including the query syntax you actually use.

@daedalus28
Copy link

daedalus28 commented Feb 13, 2025

So if I understand right, the recommended path is for users to install typebox themselves to construct schemas. Does that mean we should avoid using getValidator and querySyntax from @feathersjs/typebox as well? Or are they safe to mix?

Assuming this is true, it would be great to update the docs to reflect it.

@daffl
Copy link
Member Author

daffl commented Feb 13, 2025

getValidator will work but querySyntax is where the compatibility issue is happening.

There was an attempt for compatibility in 962bd87 - I haven't tested it with newer versions of Typebox.

@daedalus28
Copy link

Got it, so we need to construct our own query syntax or use Type from @feathers/typebox for query based schemas and are free to import from the latest typebox otherwise. I think a note about that should be added to the docs.

The commit you linked was merged. Is there currently a unit test in the repo that was known to fail when upgrading? Like I said, I'm happy to assist here - but since I don't fully understand what part of querySyntax would break, I'm not sure where to begin.

@daffl
Copy link
Member Author

daffl commented Feb 14, 2025

The commit was merged but reverted later since it caused the issue described here. I put up a new PR to the latest TypeBox in #3565 might be worth iterating there.

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

No branches or pull requests

2 participants