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

Update esbuild.js #3480

Merged
merged 3 commits into from
Jan 6, 2024
Merged

Update esbuild.js #3480

merged 3 commits into from
Jan 6, 2024

Conversation

craig-riecke
Copy link
Contributor

@craig-riecke craig-riecke commented Jan 5, 2024

This is probably a naive fix, but it does make the extension work again in my copy of VSCode.

Fixes #3171

A possibly naive way to make vscode extension work after esbuild upgrade.
Copy link

changeset-bot bot commented Jan 5, 2024

🦋 Changeset detected

Latest commit: 56f0851

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
vscode-graphql-execution Patch
vscode-graphql 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

Copy link

linux-foundation-easycla bot commented Jan 5, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: craig-riecke / name: Craig Riecke (fe7a354, 76a0a85)
  • ✅ login: acao / name: Rikki Schulte (56f0851)

@acao
Copy link
Member

acao commented Jan 6, 2024

oh great, this approach wasn't working before. did you test with URL schema or sdl file schema or both? and you tested by installing the locally generated vsix bundle?

Turns out I was quite naive about just excluding graphql-config.  The extension worked under the Test Runner, but not when bundled and installed.

This change actually does work, even when the plugin is bundled and installed separately.  It uses the technique outlined in evanw/esbuild#2441
@craig-riecke
Copy link
Contributor Author

Turns out I was naive - merely making graphql-config an external package stopped that particular error, and worked in VSCodes extension runner, but failed when I bundled the extension and installed it on its own.

This fix actually does work, inspired by evanw/esbuild#1492. I exported it as a .vsix bundle, installed it locally, and tried it with both URL and file-based Graphql Schemas, and it worked in every case.

@acao
Copy link
Member

acao commented Jan 6, 2024

@craig-riecke awesome, thank you for looking it into me! this is the last piece we needed. now i can add an e2e test suite hopefully!

@acao
Copy link
Member

acao commented Jan 6, 2024

@craig-riecke also can you add a changeset so we can kick off a patch release for this? perhaps even a minor release now that it's working again (new feature: it works 😆 )

Copy link

codecov bot commented Jan 6, 2024

Codecov Report

Attention: 30 lines in your changes are missing coverage. Please review.

Comparison is base (2348641) 55.75% compared to head (76a0a85) 55.81%.
Report is 39 commits behind head on main.

❗ Current head 76a0a85 differs from pull request most recent head 56f0851. Consider uploading reports for the commit 56f0851 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3480      +/-   ##
==========================================
+ Coverage   55.75%   55.81%   +0.06%     
==========================================
  Files         110      110              
  Lines        5243     5273      +30     
  Branches     1426     1439      +13     
==========================================
+ Hits         2923     2943      +20     
- Misses       1897     1901       +4     
- Partials      423      429       +6     
Files Coverage Δ
packages/graphiql-react/src/editor/context.tsx 73.39% <100.00%> (ø)
...raphql-language-service-server/src/GraphQLCache.ts 50.88% <50.00%> (ø)
...nguage-service/src/utils/getVariablesJSONSchema.ts 94.59% <98.38%> (+0.71%) ⬆️
packages/graphiql/src/components/GraphiQL.tsx 68.70% <40.00%> (-1.23%) ⬇️
...ql-language-service-server/src/MessageProcessor.ts 45.83% <55.00%> (-0.40%) ⬇️
packages/graphiql-react/src/editor/hooks.ts 40.15% <23.52%> (+1.56%) ⬆️

@acao
Copy link
Member

acao commented Jan 6, 2024

works for me as well on vsix bundle! awesome to see

now i see a bug though where it reports a successful response as an error, so I can fix that in another PR

update: there was a schema issue, looks great!

@acao acao merged commit a1fced1 into graphql:main Jan 6, 2024
2 of 3 checks passed
@acao acao mentioned this pull request Jan 6, 2024
@craig-riecke craig-riecke deleted the issue-3171 branch January 6, 2024 20:52
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.

vscode-graphql-execution not running
2 participants