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

prettify getting-started/index.mdx and guides/graphql-server-apollo-yoga.mdx #7707

Merged
merged 2 commits into from
Mar 30, 2022

Conversation

@changeset-bot
Copy link

changeset-bot bot commented Mar 29, 2022

⚠️ No Changeset found

Latest commit: ca90c3b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Mar 29, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/theguild/graphql-code-generator/9UYg13vJ4SUNC1mrC4mHWRfEpyVV
✅ Preview: https://graphql-code-generator-git-improve-docs-theguild.vercel.app

@@ -76,22 +73,22 @@ interface PostQuery {
}[]
}

const postsQueryDocument = `
const postsQueryDocument = /* GraphQL */ `
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adding /* GraphQL */ comments helps prettier understand that this is not string but graphql document, so he will prettify

@theguild-bot
Copy link
Collaborator

theguild-bot commented Mar 29, 2022

The latest changes of this PR are not available as alpha, since there are no linked changesets for this PR.

@@ -157,36 +149,38 @@ const Posts = () => {
</div>
</template>

<script>
<script lang="ts">
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think lang was missing, after adding this prettier prettified content inside script tag

</script>

<ul>
{ /* UI */ }
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jycouet does it valid syntax in svelte? I think comment must be <!-- UI -->

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right { /* UI */ } will not work.
If you want a comment in your HTML, you put HTML comment tag like: <!-- UI -->

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While looking at this guide, could we use KitQL instead of Svelte-Apollo in the example 😇?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dotansimha what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can do both but we have to keep in mind that guides are here to help most people get started with popular stacks

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI:
https://github.com/timhall/svelte-apollo => It's without code gen
https://github.com/ticruz38/graphql-codegen-svelte-apollo => is the codegen for 👆 svelte-apollo

Adding in the list will probably not fit.


const typeDefs = readFileSync('./schema.graphql').toString('utf-8')
const typeDefs = readFileSync('./schema.graphql', 'utf8')
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.toString('utf-8') is redundant, utf8 can be passed as 2nd argument to readFileSync()

@@ -540,6 +522,6 @@ Then, you can either read a guide or go over [the list of available plugins](/pl
If you are experiencing any issues, you can reach us via the following channels:

- Found a bug? [report it in our GitHub repo](https://github.com/dotansimha/graphql-code-generator)
- Need help or have a question? You can use the live chat box in the corner of the screen, [ask it in our GitHub Discussions page](https://github.com/dotansimha/graphql-code-generator/discussions) or [reach out to us directly in our Discord](http://bit.ly/guild-chat).
- Need help or have a question? You can use the live chat box in the corner of the screen, [ask it in our GitHub Discussions page](https://github.com/dotansimha/graphql-code-generator/discussions) or [reach out to us directly in our Discord](http://bit.ly/guild-chat)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need dot as it's list

@@ -124,7 +125,7 @@ Assuming that, as recommended, your `package.json` has the following script:
```json
{
"scripts": {
"generate": "graphql-codegen",
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need comma as it's json

@dimaMachina dimaMachina marked this pull request as ready for review March 29, 2022 23:12
Copy link
Contributor

@jycouet jycouet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we leave the svelte update on the side to not mix with this PR?


- **less boilerplate** (thanks to full code generation such as React hooks generation)

- **autocompletion on all queries, mutations and, subscription variables and results**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually not true in Svelte.
Need ardatan/graphql-tools#4349 and a PR (that I will do now on vscode)

@dimaMachina dimaMachina merged commit 1837961 into master Mar 30, 2022
@dimaMachina dimaMachina deleted the improve-docs branch March 30, 2022 11:51
@jycouet
Copy link
Contributor

jycouet commented Mar 30, 2022

Nice one 👌

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

4 participants