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

feat: support for graphql files #640

Merged
merged 6 commits into from
Jun 21, 2024
Merged

feat: support for graphql files #640

merged 6 commits into from
Jun 21, 2024

Conversation

ematipico
Copy link
Member

Summary

This PR updates the frontend to support .graphql and .gql files, however the playground is throwing an error.

This is probably a bug in core, I will look after it.

Copy link

netlify bot commented Jun 20, 2024

Deploy Preview for biomejs ready!

Name Link
🔨 Latest commit 92041cd
🔍 Latest deploy log https://app.netlify.com/sites/biomejs/deploys/6675316ab1a9990008af3b87
😎 Deploy Preview https://deploy-preview-640--biomejs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@Sec-ant
Copy link
Member

Sec-ant commented Jun 20, 2024

We need to exlude graphql for the control flow graph:

const controlFlowGraph = !(
isJsonFilename(filename) || isCssFilename(filename)
)
? workspace.getControlFlowGraph({
path,
cursor: cursorPosition,
})
: "";

@ematipico ematipico requested a review from Sec-ant June 20, 2024 15:39
@Sec-ant
Copy link
Member

Sec-ant commented Jun 20, 2024

I just realized we might have to exlude graphql from all the capabilities including formatting and linting, not only control flow graph. Otherwise those workspace. functions will throw errors:

const formatterIr = workspace.getFormatterIr({
path,
});
const importSorting = workspace.organizeImports({
path,
});
const categories: RuleCategories = [];
if (configuration?.formatter?.enabled) {
categories.push("Syntax");
}
if (configuration?.linter?.enabled) {
categories.push("Lint");
}
const diagnosticsResult = workspace.pullDiagnostics({
path,
categories: categories,
max_diagnostics: Number.MAX_SAFE_INTEGER,
only: [],
skip: [],
});
const printer = new DiagnosticPrinter(path.path, code);
for (const diag of diagnosticsResult.diagnostics) {
printer.print_verbose(diag);
}
const printed = workspace.formatFile({
path,
});

@ematipico
Copy link
Member Author

True, but would consider this an optimisation, not a blocker

@Sec-ant
Copy link
Member

Sec-ant commented Jun 20, 2024

True, but would consider this an optimisation, not a blocker

Yeah, but this problem prevents us from using graphql code in the playground, no?

@ematipico
Copy link
Member Author

Good call on this one. I applied your suggestions and now we are getting the correct CST :)

@ematipico
Copy link
Member Author

I am going to merge this, then land biomejs/biome#3248 and then I will make another PR here to enable pullDiagnostics. That should give us a good output for the diagnostics.

@ematipico ematipico merged commit c9abe0e into main Jun 21, 2024
6 checks passed
@ematipico ematipico deleted the feat/parse-graphql branch June 21, 2024 08:01
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