-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Format embedded GQL in template literal statements #16064
Conversation
a72ee69
to
b221654
Compare
My description of this feature is probably bad, feel free to fix :) |
Can you share a real world use case? |
I did in #16007. |
I realized that probably needs clarification (my apologies), so here is a real-life example: // card.tsx
import type { ContentItem } from './card.generated';
/* GraphQL */ `
fragment ContentItem on ContentItem {
id
name
thumbnailUri
}
`;
export interface ContentItemCardProps {
contentItem: ContentItem;
children: ReactNode;
}
export const ContentItemCard = (props: ContentItemCardProps) => ... This kind of construct is probably not useful at all in JS, but with TS + GQL is very useful for us. Using The template literals themselves are basically dead code and can be removed from bundle output. An alternative would be to drop a graphql file alongside Please lemme know if I need to clarify any more. |
Also maybe worth noting that all my other tooling (codegen, GQL LSP) supports this use case well. It's just a remaining nit that I have to format it by hand right now. |
src/language-js/embed/utils.js
Outdated
(parent.type === "ExpressionStatement" && | ||
node.type === "TemplateLiteral" && | ||
hasLeadingBlockCommentWithName(parent, languageName)) |
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.
(parent.type === "ExpressionStatement" && | |
node.type === "TemplateLiteral" && | |
hasLeadingBlockCommentWithName(parent, languageName)) | |
(parent.type === "ExpressionStatement" && | |
hasLeadingBlockCommentWithName(parent, languageName)) |
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.
Thanks - I think I realized that check wasn't strictly necessary, but I just don't know enough about how things work here, so in my mind I was playing it safe somehow. I'll update this today.
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.
Also, would a better description be: Format embedded GQL in template literal statements
?
b221654
to
2826b32
Compare
Anything else I need to do here? |
Sorry for the delay, will merge when CI goes through. |
Description
Fixes #16007
Checklist
docs/
directory).changelog_unreleased/*/XXXX.md
file followingchangelog_unreleased/TEMPLATE.md
.✨Try the playground for this PR✨