Change default recommended configuration file format to `json`. by phryneas · Pull Request #209 · apollographql/vscode-graphql

phryneas · 2024-09-17T13:15:51Z

This changes the default recommended configuration file format to .json.

I would prefer to make this yaml instead, but the configuration's JSON schema is natively supported by VSCode for .json files and needs an additional extension to be installed by the user to support .yaml files, so I believe going this way will result in less onboarding friction.

handle `apollo.config.json` as jsonc.

github-actions · 2024-09-17T13:16:46Z

You can download the latest build of the extension for this PR here:
vscode-apollo-0.0.0-build-1726646915.pr-209.commit-96c069e.zip.

To install the extension, download the file, unzip it and install it in VS Code by selecting "Install from VSIX..." in the Extensions view.

Alternatively, run

code --install-extension vscode-apollo-0.0.0-build-1726646915.pr-209.commit-96c069e.vsix --force

from the command line.

For older builds, please see the edit history of this comment.

phryneas · 2024-09-17T13:23:51Z


 <h2 id="getting-started">Getting started</h2>

+For the VS Code plugin to know how to find the schema, it needs to be linked to either a published schema or a local one.


@Meschreiber, could I ask you to take a quick look at these changes? Once we ship it here and the details tags are working, I would add the same changes to the corresponding Docs page.

I made these changes as we get a lot of reviews that complain that you need a GraphOS account, which is not true. It seems the alternative configurations for local service and local file were not prominent enough until now.

By making these sections collapsible, I'm trying to make all of them more discoverable.

Thanks for the ping here!

I didn't realize the README for this project had a lot of duplicated content with the docs page. Did we consider just linking to the docs page rather than maintaining both places?/Do you think the benefit of visibility (particularly in the VSC marketplace) outweighs the minor burden of maintaining in both places?

Yes, definitely - especially the screenshots help a lot showcasing the features we have. I'm happy to keep both in sync :)

phryneas · 2024-09-17T13:46:09Z

+    build.onResolve(
+      { filter: /^jsonc-parser$/ },
+      async ({ path, ...options }) => {
+        return build.resolve("jsonc-parser/lib/esm/main.js", options);


This package offers ESM and UMD. If I import the UMD in VSCode (the default), it makes the extension crash - if I import the ESM in the tests, the tests crash.
This way, the tests get UMD and the extension bundles with the ESM import.

phryneas · 2024-09-17T13:55:38Z

-module.exports = {
-  client: {
-    tagName: "graphql", // highlight-line
-    service: ...


This actually rendered out in the Marketplace, so I'm removing it

Meschreiber

Just some structural suggestions and a question. Otherwise LGTM!

Meschreiber · 2024-09-17T19:52:15Z


 <h2 id="getting-started">Getting started</h2>

+For the VS Code plugin to know how to find the schema, it needs to be linked to either a published schema or a local one.


Thanks for the ping here!

I didn't realize the README for this project had a lot of duplicated content with the docs page. Did we consider just linking to the docs page rather than maintaining both places?/Do you think the benefit of visibility (particularly in the VSC marketplace) outweighs the minor burden of maintaining in both places?

Meschreiber · 2024-09-17T19:55:41Z


-1. Create an `apollo.config.js` file at the root of the project.
-   Alternatively, you can create a `cjs`, `mjs`, or `ts` file with the same configuration.
+1. Create an `apollo.config.json` file at the root of the project.


Seems like this step is no longer necessary since it's mentioned above?

Yeah, and this is only a summary of what's coming next, so I'll delete the whole thing.

Meschreiber · 2024-09-17T19:58:59Z

+
+<details>
+<summary>
+<h3>Setting the Extension up for use with Apollo GraphOS</h3>


I think it's important that the title clearly states this is the "published" option.

Suggested change

<h3>Setting the Extension up for use with Apollo GraphOS</h3>

<h3>Configure extension for schemas published to Apollo GraphOS</h3>

Meschreiber · 2024-09-17T20:01:28Z

 <h3 id="apollo-config">Setting up an Apollo config</h3>

-For the VS Code plugin to know how to find the schema, it needs to be linked to either a published schema or a local one. To link a project to a published schema, edit the `apollo.config.js` file to look like this:
+To link a project to a published schema, edit the `apollo.config.json` file to look like this:

-```js
-module.exports = {
-  client: {
-    service: "my-graphql-app",
-  },
-};
+```json
+{
+  "client": {
+    "service": "graphos-graph-id"
+  }
+}


I would remove this and the following <h3> and just have one continuous set of instructions. E.g.,

Obtain a personal API key.

Edit the apollo.config.json to look like this...

To pull down your schema, create an .env file ...

Meschreiber · 2024-09-17T20:02:07Z

-To authenticate with GraphOS Studio to pull down your schema, create a `.env` file in the same directory as the `apollo.config.js` file. This should be an untracked file (that is, don't commit it to Git).
+To authenticate with GraphOS Studio to pull down your schema, create a `.env` file in the same directory as the `apollo.config.json` file. This should be an untracked file (that is, don't commit it to Git).

 Then go to your [User Settings page](https://studio.apollographql.com/user-settings/api-keys?referrer=docs-content) in GraphOS Studio to create a new Personal API key.


Is this different than the API key they needed to previously obtain?

No, the exact same one. This whole thing is just a bit too repetitive 😅
Removed the earlier mention.

Meschreiber · 2024-09-17T20:02:57Z

+
+<details>
+<summary>
+<h3 id="local-schemas">Setting the Extension up for use with a local schema</h3>


Suggested change

<h3 id="local-schemas">Setting the Extension up for use with a local schema</h3>

<h3 id="local-schemas">Configure extension for local schemas</h3>

Meschreiber · 2024-09-17T20:07:06Z

+</details>
+
+<details>
+<summary>


I would include this under the same header (Configure extension for local schemas) and present the two options has <h4>s.

Use introspection of a locally running service

Use with a local schema file.

IMO, since these are two flavors of the "Configure extension for local schemas" they should be separate <h4>s within that collapsible section.

People have been missing these options a lot, so I want to have both options as visible as the GraphOS one. It's really pulling down our marketplace ratings and leads to bad reviews from non-GraphOS users (which make up a big part of our userbase - we have 500k downloads).

jerelmiller

Spotted one stray log, but other than Maria's requests, I've got nothing else.

jerelmiller · 2024-09-17T21:50:19Z

+const resolvePlugin = {
+  name: "resolve",
+  setup(build) {
+    console.log("setup");


Suggested change

console.log("setup");

Is this needed?

Not anymore, good catch :D

Meschreiber

Thanks for restructuring, looks a lot clearer!

phryneas added 3 commits September 17, 2024 14:43

update README to reflect .json as default configuration extension

684bd54

use jsonc-parser for json config files

bc37a7e

handle `apollo.config.json` as jsonc.

changeset

70b79e9

phryneas requested a review from jerelmiller September 17, 2024 13:21

phryneas commented Sep 17, 2024

View reviewed changes

adjust build to not break tests

ca60a0b

phryneas requested a review from Meschreiber September 17, 2024 13:39

also test for jsonc in test

e9b8593

phryneas commented Sep 17, 2024

View reviewed changes

remove //highlight-line comment

759269d

phryneas commented Sep 17, 2024

View reviewed changes

phryneas requested a review from alessbell September 17, 2024 15:43

Meschreiber approved these changes Sep 17, 2024

View reviewed changes

jerelmiller approved these changes Sep 17, 2024

View reviewed changes

phryneas added 3 commits September 18, 2024 10:05

review feedback

61b329e

adjust changeset

509e776

Merge branch 'main' into pr/default-config-json

827bab4

phryneas requested a review from Meschreiber September 18, 2024 08:08

Meschreiber approved these changes Sep 18, 2024

View reviewed changes

phryneas merged commit fc73ed3 into main Sep 18, 2024

phryneas deleted the pr/default-config-json branch September 18, 2024 13:59

github-actions Bot mentioned this pull request Sep 18, 2024

Version Packages #205

Merged


		<h2 id="getting-started">Getting started</h2>

		For the VS Code plugin to know how to find the schema, it needs to be linked to either a published schema or a local one.

	<h3>Setting the Extension up for use with Apollo GraphOS</h3>
	<h3>Configure extension for schemas published to Apollo GraphOS</h3>

	<h3 id="local-schemas">Setting the Extension up for use with a local schema</h3>
	<h3 id="local-schemas">Configure extension for local schemas</h3>

Conversation

phryneas commented Sep 17, 2024

Uh oh!

github-actions Bot commented Sep 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Meschreiber left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Meschreiber Sep 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phryneas Sep 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Meschreiber Sep 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phryneas Sep 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jerelmiller left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Meschreiber left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Sep 17, 2024 •

edited

Loading

Meschreiber Sep 17, 2024 •

edited

Loading

phryneas Sep 18, 2024 •

edited

Loading

Meschreiber Sep 17, 2024 •

edited

Loading

phryneas Sep 18, 2024 •

edited

Loading