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

Only parse correctly tagged (sql...) templates #85

Merged
merged 1 commit into from
Dec 2, 2023

Conversation

andersthuesen
Copy link
Contributor

@andersthuesen andersthuesen commented Nov 29, 2023

First and foremost, thank you for creating this project! 🔥
Tried it out with our codebase at work, and got a lot of errors just from template strings that were not meant for SQL queries.
Example:

[INFO] Scanning "./pages/api" for SQLs with extension Ts
checking generate types config None
error: internal compiler error: syntax error at or near "FAIL"
  --> <./pages/api/post/is_empty.ts>:16:9
   |
16 |         const test = `FAIL HAHAHA`;
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^

Turning off query registration for Tpl expressions seems to do the trick!
Please let me know if you think this is a good idea 😄

@JasonShin
Copy link
Owner

JasonShin commented Dec 2, 2023

Thanks for this PR, let me give it a try. That's weird, I've never got this error on my end. it could be that I've naively never tested against having some `some strings` strings in the tests and demo haha..

Reading the code, I wonder why did I do that there as I'm already extracting SQLs from get_sql_from_expr method 🤯

@JasonShin
Copy link
Owner

JasonShin commented Dec 2, 2023

Hello, I tested the changes and it's all good. I will fix the failing tests after merging. It won't trigger any releases 👍

Thanks for your first contribution! (how did I miss this? #79)

The follow-up PR is #86

Please try v0.9.1+ for the fix

@JasonShin JasonShin merged commit d4e084b into JasonShin:main Dec 2, 2023
9 of 10 checks passed
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.

2 participants