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

debounce schema change events to fix codegen bugs #3647

Merged
merged 6 commits into from
Jul 9, 2024

Conversation

acao
Copy link
Member

@acao acao commented Jul 6, 2024

on mass file changes, network schema is requesting schema way too frequently because the schema cache is invalidated on every schema file change

to address this, we debounce the onSchemaChange event by 400ms

also, fix bugs with tests, and schemaCacheTTL setting not being passed to the cache

This also includes an integration test to confirm schema changes over the network! 🥳

this should address #3622 properly

Copy link

changeset-bot bot commented Jul 6, 2024

🦋 Changeset detected

Latest commit: 1930fb5

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

This PR includes changesets to release 3 packages
Name Type
graphql-language-service-server Patch
graphql-language-service-cli 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

@acao acao added bug lsp-server graphql-language-service-server lsp cli vscode-graphql labels Jul 6, 2024
@acao acao force-pushed the debounce-schema-change-events branch 2 times, most recently from ed89b00 to 7313ff7 Compare July 6, 2024 19:26
Copy link

codecov bot commented Jul 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.86%. Comparing base (e33af28) to head (1930fb5).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3647      +/-   ##
==========================================
+ Coverage   60.80%   60.86%   +0.06%     
==========================================
  Files         120      120              
  Lines        5620     5621       +1     
  Branches     1489     1492       +3     
==========================================
+ Hits         3417     3421       +4     
+ Misses       1750     1748       -2     
+ Partials      453      452       -1     
Files Coverage Δ
...raphql-language-service-server/src/GraphQLCache.ts 74.16% <100.00%> (+0.09%) ⬆️
...ql-language-service-server/src/MessageProcessor.ts 75.43% <100.00%> (+0.65%) ⬆️

@acao acao force-pushed the debounce-schema-change-events branch from 7313ff7 to b21282e Compare July 6, 2024 19:38
on mass file changes, network schema is requesting schema way too
frequently because the schema cache is invalidated on every schema file
change

to address this, we debounce the onSchemaChange event by 400ms

also, fix bugs with tests, and schemaCacheTTL setting not being passed
to the cache
@acao acao force-pushed the debounce-schema-change-events branch from b21282e to af3102a Compare July 6, 2024 19:47
Copy link
Contributor

github-actions bot commented Jul 6, 2024

The latest changes of this PR are available as canary in npm (based on the declared changesets):

@acao acao force-pushed the debounce-schema-change-events branch 2 times, most recently from c484615 to 4b3abcc Compare July 7, 2024 08:53
@acao acao force-pushed the debounce-schema-change-events branch from fd39c2c to ae40378 Compare July 7, 2024 10:03
Copy link
Contributor

@TallTed TallTed left a comment

Choose a reason for hiding this comment

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

capitalized some acronyms, for consistency and clarity

packages/graphql-language-service-server/README.md Outdated Show resolved Hide resolved
packages/vscode-graphql/README.md Outdated Show resolved Hide resolved
@acao acao merged commit ba5720b into main Jul 9, 2024
14 checks passed
@acao acao deleted the debounce-schema-change-events branch July 9, 2024 14:36
@acao acao mentioned this pull request Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug lsp cli lsp-server graphql-language-service-server vscode-graphql
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants