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

fix: Don't load localSchemaFile as part of project code #8

Merged
merged 4 commits into from
Aug 1, 2021

Conversation

jgzuke
Copy link
Contributor

@jgzuke jgzuke commented Jul 28, 2021

This should fix what I believe is the most common case for hitting apollographql/apollo-tooling#801 (VSCode Plugin: Loading Schema Twice).

When using a local schema file to power the VSCode extension, if the schema file is in the includes (which it probably is by default), we will read it as part of the project code and try to extend the existing schema. Since this file has already been included as the base, the extension will fail and show an error.

This change adds the local schema file(s) to the excludes list for the file set, which I think this makes sense overall. Excluding this from the project does mean that it wont get included for providers when editing this file, however in practice I dont think this worked anyways, and I dont see a strong use case for supporting editing a local schema file anyways assuming it did work.

First three commits are refactors to make the intent of the final commit more clear.

@jgzuke jgzuke requested a review from cy July 28, 2021 10:25
Base automatically changed from jgzuke/misc/typescript-fixes to main July 29, 2021 02:41
jgzuke added 4 commits July 28, 2021 19:42
This wasn’t used, and also didn’t seem like it had the correct value anyways (didn’t account for `config.configDirURI`)
This seemed a bit confusing given that the `rootURI` means the project root elsewhere. Let’s call this what is it since it wont always match the actual root uri
@jgzuke jgzuke force-pushed the jgzuke/bugs/loading-schema-twice branch from 03e4f0a to 5b50b28 Compare July 29, 2021 02:42
Copy link
Contributor

@cy cy left a comment

Choose a reason for hiding this comment

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

makes sense, and still runs so :shipit:

@jgzuke jgzuke merged commit c7b6679 into main Aug 1, 2021
@jgzuke jgzuke deleted the jgzuke/bugs/loading-schema-twice branch August 1, 2021 08:11
@github-actions github-actions bot mentioned this pull request Oct 16, 2023
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.

2 participants