-
Notifications
You must be signed in to change notification settings - Fork 331
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
Enable typed queries for threads #1568
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Ignored Deployments
|
Deployment failed with the following error:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a few comments. Overall, I think the safest way to write a function that takes a more complex object and produces a valid query string is to use a recursive "prettifier", which takes an AST (in this case the "typed query", which sort of is a simplified AST) and produces a string value, i.e. the reverse operation of a parse.
You can see an example of it here:
https://github.com/liveblocks/liveblocks-cloudflare/blob/main/packages/liveblocks-query-parser/src/prettify/index.ts
With a function like that, it's impossible to generate syntactically invalid outputs, and it will take care of all the proper escaping. (Notice how this function calls itself recursively on sub parts of the AST, until it reaches the "nodes", which it basically just uses JSON.stringify()
for.)
@FlowFlorent Let me know once you're done with this PR. I'm happy to play devils advocate and try to come up with unit tests that won't pass. |
@nvie Thanks for all your comments, I've adjusted the PR. I am still a bit uncertain about the escaping part though 🤔 |
Deployment failed with the following error:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, @FlowFlorent! 🙌
@@ -45,6 +45,7 @@ | |||
"devDependencies": { | |||
"@liveblocks/eslint-config": "*", | |||
"@liveblocks/jest-config": "*", | |||
"@liveblocks/query-parser": "^0.0.3", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"@liveblocks/query-parser": "^0.0.3", | |
"@liveblocks/query-parser": "^0.0.4", |
Description
@liveblocks/react
useThreads
now handles typed queriesExample:
@liveblocks/node
getThreads
now handles typed queries