Repository navigation
Conversation
12e4586 to
2ef3053
Compare
ilbertt
left a comment
There was a problem hiding this comment.
Hey @haydn, thanks for the PR!
I think we should just have one way to type the query to avoid introducing complexity and more documentation. I'm not attached to the current named tag approach, and I really like the zero runtime advantage that your approach would introduce.
What I like about the current named tag approach is that the flow feels more "natural": I assign the name to the query, and the generator picks it up to generate its type. In your approach, the generator creates the type because you referenced it, which feels weird.
Also, I'm a bit scared that we would have to handle many edge cases like type aliases (e.g. import type { Queries as Q }...).
What are your thoughts?
By the way, #40 moved all the advanced cases from the simple (which has also been renamed into "basic" in #42) example to the advanced example, in case you want to update your branch.
Yeah, I totally agree. Two ways of doing it would add needless complexity.
They both feel a bit weird IMO. Either way your code is invalid until the generator runs because whatever you're writing is the canonical reference (slight caveat: in the typed version of this PR the code would be executable, but have invalid types). Some prior art I know of is TanStack Router's Another idea I had (and I'm not sure it's even viable) would be to use the query itself as the way of identifying it (so the developer doesn't need to name it at all). The idea is the types that are generated overload Bun's type sql = (strings: ["SELECT 'foo' as bar"]) => Promise<{ bar: string }[]>;
Yeah, that's a valid concern. I guess that problem can exist irrespective of the approach though? For example, someone can do something like this: import { SQL as Blah } from "bun";
const { MyQuery } = withTypes(new Blah(DATABASE_URL));
const result = await MyQuery`SELECT now()`;
Got you. I'll update this PR when I get a chance. |
|
Thinking about it more, I’m leaning toward your native generics as the single approach, especially for the zero runtime dependencies.
Fair point!
Yeah it would be nice but I think this is not possible with the current version of TypeScript (see microsoft/TypeScript#33304)
Yeah, I should have been more specific: Could we handle that import alias and verify discovery still works before the generated file exists? Those are the main cases I’d want to check before moving toward native generics as the single approach. |
Dang. Yeah, that's a nonstarter.
Got you. This sounds doable. I should have some time to take a look into it this weekend. |
|
@haydn any update on this PR? |
2ef3053 to
fb97144
Compare
|
@ilbertt Apologies, I've been meaning to get to this. I've just pushed-up a rebased version with a WIP commit. Does the gist of that look like what you had in mind? |
Hey @ilbertt. Love the tool. This PR is a bit of an experiment/proposal for an alternative way of discovering the SQL tagged-templates.
The gist of it is this:
IMO, there are two advantages to this approach:
bun-sqlgenwould be a dev-only dependency).sql<Type>`SELECT now()`pattern is better recognised by other tooling (syntax highlighters, formatters, code completions etc).