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(gatsby): correctly replace exported queries for StaticQuery #11545

Conversation

DSchau
Copy link
Contributor

@DSchau DSchau commented Feb 4, 2019

Description

This PR fixes the static query extraction (rather the import statement)
failing when the query is exported. The issue was that the AST was
expected to be at a certain depth (e.g. the .parentPath stuff), and we
need to go up one more level to get the correct element we expected,
i.e. the component.

I also added an e2e test to guard against this behavior in the future.

Related Issues

Fixes #11477

This PR fixes the static query extraction (rather the import statement)
failing when the query is `export`ed. The issue was that the AST was
expected to be at a certain depth (e.g. the .parentPath stuff), and we
need to go up _one_ more level to get the correct element we expected,
i.e. the component.

Fixes gatsbyjs#11477
@DSchau DSchau requested a review from a team as a code owner February 4, 2019 19:37
@@ -76,12 +83,12 @@ function removeImport(tag) {
else importPath.remove()
}
if (importPath.isObjectProperty()) {
if (parent.node.properties.length === 1)
importPath.findParent(p => p.isVariableDeclaration())?.remove()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These were causing eslint to go wild. I lightly refactored this and used a helper.

@DSchau
Copy link
Contributor Author

DSchau commented Feb 4, 2019

I used this example to validate this PR. Doesn't seem to cause any regressions as best as I can tell, just fixes the bug.

@KyleAMathews
Copy link
Contributor

Nice! Hopefully this is the main bug that people keep hitting with static queries.

@DSchau
Copy link
Contributor Author

DSchau commented Feb 4, 2019

@KyleAMathews I think this is one of them. The other is the one that @tkh44 @jlengstorf seem to be intermittently hitting, i.e. something like cannot find relative path '../../public/static/d/asdfasdfs.json'.

I think that's a monorepo/workspaces bug though, not necessarily something with StaticQuery. Still need to research it more though.

Copy link
Contributor

@pieh pieh left a comment

Choose a reason for hiding this comment

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

Nice one @DSchau!

@pieh pieh merged commit 9c9607a into gatsbyjs:master Feb 4, 2019
@DSchau
Copy link
Contributor Author

DSchau commented Feb 4, 2019

Successfully published:
 - [email protected]
 - [email protected]

@DSchau DSchau deleted the babel-plugin-remove-graphql-queries/export-static-query branch February 4, 2019 23:36
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.

[bug]: StaticQuery fails when export is used to export query
3 participants