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

relint when schema changes #3405

Merged
merged 3 commits into from
Aug 19, 2023
Merged

Conversation

imolorhe
Copy link
Contributor

@imolorhe imolorhe commented Aug 12, 2023

Relint when schema changes. Codemirror 6 normally only runs linting whenever the document changes. However this means whenever the GraphQL schema changes, the linting is not run again until the document changes. This can be lead to confusing behaviors.

A new option was added to the linter recently that allows configuring a predicate to determine when the linter will be ran when the state changes. We are leveraging the same here to trigger linting whenever the schema or passed option changes.

Note: linting always runs on document changes irrespective of what we define in this predicate, which is what we want.

Related issue: altair-graphql/altair#2242

@changeset-bot
Copy link

changeset-bot bot commented Aug 12, 2023

🦋 Changeset detected

Latest commit: 7f2493a

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

This PR includes changesets to release 1 package
Name Type
cm6-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

@codecov
Copy link

codecov bot commented Aug 12, 2023

Codecov Report

Merging #3405 (7f2493a) into main (2348641) will increase coverage by 0.10%.
Report is 10 commits behind head on main.
The diff coverage is 98.38%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3405      +/-   ##
==========================================
+ Coverage   55.75%   55.85%   +0.10%     
==========================================
  Files         110      110              
  Lines        5243     5256      +13     
  Branches     1426     1432       +6     
==========================================
+ Hits         2923     2936      +13     
  Misses       1897     1897              
  Partials      423      423              
Files Changed Coverage Δ
...nguage-service/src/utils/getVariablesJSONSchema.ts 94.59% <98.38%> (+0.71%) ⬆️

@github-actions
Copy link
Contributor

github-actions bot commented Aug 12, 2023

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

@dimaMachina dimaMachina merged commit 3d4b9b7 into main Aug 19, 2023
12 checks passed
@dimaMachina dimaMachina deleted the imolorhe/cm6-relint-on-schema-change branch August 19, 2023 00:51
@acao acao mentioned this pull request Aug 19, 2023
@imolorhe
Copy link
Contributor Author

@acao I was trying to get the example package working but I was running into some issues with the duplicate @codemirror/state dependency instances which is why I haven't done that yet. We may have to move the example into the package itself like we did in codemirror-json-schema

@acao
Copy link
Member

acao commented Aug 19, 2023

i got around that, in the json schema mode with the same issue. big pr coming there haha

@imolorhe
Copy link
Contributor Author

Oh nice. Can't wait to see what you come up with!

@acao
Copy link
Member

acao commented Aug 19, 2023

@imolorhe i was able to make this error go away by moving away from basicSetup package, whether it came from codemirror or @codemirror/basic-setup

here is a PR!
https://github.com/acao/codemirror-json-schema/pull/63/files

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.

3 participants