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

Add sql template literals support via plugins #12139

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ivank
Copy link

@ivank ivank commented Jan 24, 2022

Description

Add support for prettifying sql template literals.

import { sql } from '@potygen/potygen';

const myQuery = sql`
  SELECT * FROM table1
`;

This does not add the support itself, just the ability to do so by plugins, if they provide the "sql" parser/printer. Does not change any behaviour if no plugins like that are present.


I'm developing a Postgres sql typescript type generator - potygen. As part of that effort I've ended up with a pretty decent pure JS sql parser and figured - lets do a sql prettier plugin. And that turned out working great too.

To my horror though 😱 all of our sql template literals in our service didn't magically become pretty! Reading the issues and source it quickly became obvious that there are only several blessed tags that are processed, and my newly minted prettifier would be delegated to the cold wintery post of just dealing with raw sql files. That's great and all, but I was craving for more!

I'm also not the only one figuring "raw sql is better than ORM", and indeed I've found a lot of like minded folk along the way - squid, ts-mysql-analyzer, pgtyped and I'm sure many others, so the need and desire for sql template literals is here, we just need prettier to allow us to plug that in.

Checklist

  • I’ve added tests to confirm my change works.
  • (If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory).
  • (If the change is user-facing) I’ve added my changes to changelog_unreleased/*/XXXX.md file following changelog_unreleased/TEMPLATE.md.
  • I’ve read the contributing guidelines.

Try the playground for this PR

@ivank
Copy link
Author

ivank commented Jan 24, 2022

Made a quick package for our local development that incorporates those patches https://www.npmjs.com/package/@ikerin/prettier - will try to keep it up-to-date

@fisker
Copy link
Member

fisker commented Jan 25, 2022

I'm afraid we are not going to accept this before we find a better solution. See #7573 #7576 #6626

@ivank
Copy link
Author

ivank commented Jan 25, 2022

Seems reasonable. Is there a discussion about what this fabled better solution might entail though? There's obviously a desire for more and different template literal decorators, so much so that Microsoft have themselves developed an extension mechanism for it.

Prettier already has this wonderful plugin system, it doesn't look like too much of a stretch to add some more configs to it, and maybe even implement all the other template literal embeds with it?

I'm happy to try and delve into the innards of prettier if you guys are too busy with other stuff, but I'd really appreciate some direction. I know nobody wants a huge PR bomb dropped in their lap :)

@rdsedmundo
Copy link

It's quite disappointing that the embed functionality is so limited, while you can override a parser of a language to use the original parser + some additional logic, there's no way to override the original printer. I tried to do that for the TypeScript parser, and adding a printers.estree in my custom plugin had no effect (#8195).

As it stands, the current API is almost useless, because of the indirection it creates: I have to create a plugin for the language the embedding will be insert to, instead of a plugin for the embedded language itself.

@jaydenseric
Copy link

Also /* SQL */ ` … ` should be considered an SQL template literal, like Prettier and other tools do with /* GraphQL */ ` … `. You don't always tag template literals with functions. In fact, I don't in any of my codebases.

@karlhorky
Copy link
Contributor

karlhorky commented Oct 26, 2023

For anyone finding this and looking for another alternative for formatting SQL in embedded sql tagged template literals:

Prettier can be used with the combination of the prettier-plugin-embed and prettier-plugin-sql plugins:

@karlhorky
Copy link
Contributor

karlhorky commented Oct 30, 2023

Back to this current PR, @fisker @sosukesuzuki @thorn0 what do you and Prettier team think of reconsidering this PR?

It may not be the most clean, extensible approach wished for by @ikatyang in #4424, but maybe it would be nice for at least SQL to become a first-class citizen like GraphQL and CSS.

Especially now that the industry is getting a lot of movement around sql tagged template literals thanks to:

  1. React Server Actions with SQL tagged template literals in Next.js v14 https://twitter.com/AdamRackis/status/1717607565260124613
  2. Cloudflare WebSocket-to-TCP proxy (Cloudflare blog post)
  3. Postgres.js rise in popularity https://star-history.com/#porsager/postgres&Date
  4. Vercel Postgres service + @vercel/postgres package https://vercel.com/docs/storage/vercel-postgres/sdk

If SQL tagged template literal support was first-class in Prettier core, then plugins like prettier-plugin-sql by @Shinigami92 would work out of the box for SQL in JavaScript / TypeScript instead of failing

@jpike88
Copy link

jpike88 commented Nov 2, 2023

It would be highly valuable to have some formatting being auto applied to the sql inside html template strings, relying on my devs to keep it clean isn't a guaranteed way to do it.

@sosukesuzuki
Copy link
Member

Personally, I don't want to accept SQL specific code. We should consider designing a general mechanism for using plugins with embedded formatting.

@karlhorky
Copy link
Contributor

karlhorky commented Nov 3, 2023

Personally, I don't want to accept SQL specific code.

Ok, thanks for the answer.

What makes SQL different than eg. GraphQL for you? Or do you mean that first-class Prettier support of GraphQL was also a mistake?

@sosukesuzuki
Copy link
Member

GraphQL was already supported when I became a maintainer.

@karlhorky
Copy link
Contributor

Ok, so if I'm understanding, the first-class support using the current system is the part that is unwanted (maybe the code is hard to maintain or has other problems) and should be rebuilt, meaning that any out-of-the-box support like GraphQL and CSS support will also be moved to the new system.

Would be great for users to get SQL support before this rewrite, but I understand that your position is that no new embedded languages should be added to the old, undesirable system.

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.

None yet

7 participants