graphiql
graphiql copied to clipboard
feat: add prettier!
This introduces prettier formatting by dynamically importing prettier's core library and the graphql plugin. This is how we've imported prettier in monaco-graphql's webworker compatible language service since 2020!
It's not ideal for UX, because the dependency is fetched only when using format for the first time. I would prefer to use pre-fetching, but I'm not sure how to implement import() prefetching in a cross-bundler compliant way. We could also manually prefetch on a triggered event, such as blur/focus of the format button.
🦋 Changeset detected
Latest commit: 4b4b659d90666ebacb2a6ed4c6259287275dec40
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 2 packages
| Name | Type |
|---|---|
| graphiql | Patch |
| @graphiql/react | Patch |
Not sure what this means? Click here to learn what changesets are.
Click here if you're a maintainer who wants to add another changeset to this PR
@benjie can you speak to this from the language spec perspective? is using prettier's formatting for graphql safe for us from a spec compliance perspective? graphql support is shipped with prettier core, and is a frequently maintained and updated capability as the language spec changes it would seem
Codecov Report
Merging #2508 (4b4b659) into main (2d91916) will increase coverage by
2.65%. The diff coverage is28.73%.
@@ Coverage Diff @@
## main #2508 +/- ##
==========================================
+ Coverage 65.70% 68.36% +2.65%
==========================================
Files 85 71 -14
Lines 5106 4182 -924
Branches 1631 1376 -255
==========================================
- Hits 3355 2859 -496
+ Misses 1747 1318 -429
- Partials 4 5 +1
| Impacted Files | Coverage Δ | |
|---|---|---|
| packages/codemirror-graphql/src/lint.ts | 100.00% <ø> (ø) |
|
| packages/codemirror-graphql/src/results/mode.ts | 47.05% <ø> (ø) |
|
| packages/codemirror-graphql/src/utils/hintList.ts | 95.65% <ø> (ø) |
|
| ...ckages/codemirror-graphql/src/utils/mode-indent.ts | 0.00% <0.00%> (ø) |
|
| packages/codemirror-graphql/src/variables/mode.ts | 79.48% <ø> (ø) |
|
| packages/graphiql-react/src/editor/whitespace.ts | 100.00% <ø> (ø) |
|
| packages/graphiql-react/src/utility/debounce.ts | 0.00% <0.00%> (ø) |
|
| packages/graphiql-react/src/editor/tabs.ts | 5.76% <5.76%> (ø) |
|
| ...aphiql-toolkit/src/create-fetcher/createFetcher.ts | 58.62% <50.00%> (-2.10%) |
:arrow_down: |
| ...ackages/graphiql-toolkit/src/create-fetcher/lib.ts | 56.45% <58.33%> (-3.13%) |
:arrow_down: |
| ... and 97 more |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 915b4ff...4b4b659. Read the comment docs.
The latest changes of this PR are available as canary in npm (based on the declared changesets):
[email protected]
@graphiql/[email protected]
I cannot speak on behalf of prettier, and I certainly don't know what it means for experimental features of GraphQL. I would imagine that there wouldn't be much pushback from prettier for supporting newer syntax so long as it is officially supported by graphql-js; however I do not know who is currently doing that work.
Total sidenote: I started hacking on my own GraphQL implementation in JS (more modular compared to the all-in-one graphql-js) which is storing comments on the AST nodes: https://github.com/thomasheyenbrock/graphql-modular/blob/main/packages/language/src/index.ts
This is far from being useable (implemented traversal but no printing yet), just something I wanted to drop here.
Very cool @thomasheyenbrock! I highly suggest reaching out to @dotansimha at the guild, as they have a very similar modular library!
@benjie to answer your question, prettier uses graphql-js parse and visit directly, so it honors whichever version of graphql-js you have installed. It is part of the core prettier project, and is kept up to language spec more than even graphiql by multiple contributors and maintainers to the prettier project
there is only one config option that is honored as well
imho, it would be most ideal to use a method exported from the official graphql-js project, so perhaps @saihaj , @IvanGoncharov or others could help with that. Obviously it would be most ideal to keep everything in house with the graphql org, but I worry that this will add more scope and confusion as users expect the same behavior that prettier provides as the dominant autoformatting tool in use for graphql
i don’t know how the prettier project feels about it, but perhaps they would like us to maintain it?
either way, prettier is being used to format the language spec now, so we should make sure that whatever we decide to use officially is consistent.
otherwise, people will copy paste queries from graphiql to their editor where prettier is enabled, and often need to apply more autoformatting because of the different output, which is one of the main reasons people have requested prettier formatting in graphiql. This was the reasoning for my choice to use prettier with monaco-graphql.
so whatever we decide to do, as long as it has parity with prettier‘s formatting output, it will make users happy!
we do have an issue to support prettyPrint so it is prettier like formatting https://github.com/graphql/graphql-js/issues/3230
@saihaj @thomasheyenbrock what if we „incubate“ this formatting utility in graphiql as an internal detail, and then propose it as a PR to graphql-js?
@saihaj @thomasheyenbrock what if we „incubate“ this formatting utility in graphiql as an internal detail, and then propose it as a PR to graphql-js?
graphql-js wouldn’t take this since it would require prettier as a dependency
@saihaj apologies for the confusion- we are proposing writing a custom formatter function that we maintain internally and/or contribute to graphql-js. Prettier would not be a dependency. See @thomasheyenbrock ‘s example above!
@saihaj apologies for the confusion- we are proposing writing a custom formatter function that we maintain internally and/or contribute to graphql-js. Prettier would not be a dependency. See @thomasheyenbrock ‘s example above!
Ohhhh yes that would work!
That sounds like a good proposal to me @acao! Just a custom printer that preserves comments would already be a huge improvement for GraphiQL, and we could iteratively see how to maybe incorporate that into graphql-js 👍
Does that mean we abandon this PR in favor of a custom printer, or would we still want to add prettier as a first step and do the custom printer as next iteration?
@thomasheyenbrock either is fine, we can just start over if that makes more sense to you!
It would also be tremendous to add this to monaco-graphql, graphql-language-service-server and others!