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

[v2] fix eslint warnings #5121

Merged
merged 3 commits into from
Apr 25, 2018
Merged

[v2] fix eslint warnings #5121

merged 3 commits into from
Apr 25, 2018

Conversation

m-allanson
Copy link
Contributor

build and develop works locally on gatsbygram, I've not tested these changes any further.

@@ -119,11 +118,13 @@ async function findGraphQLTags(file, text): Promise<Array<DefinitionNode>> {
TaggedTemplateExpression(path) {
if (
(`descendant of query`,
// eslint-disable-next-line no-undef
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Couldn't figure out how to get babel-eslint to like the optional chaining here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, noticed that too — it must not be setup to use or repo babel setup

Copy link
Contributor

Choose a reason for hiding this comment

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

There is one specific minor version that it works with...I'll find it I have it set in another project but don't remember off the top of my head :P

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that'd be ace, thanks :D

Copy link
Contributor

Choose a reason for hiding this comment

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

its 8.2.1 and earlier i think, you can pin to that specific version tho, any higher and it's broken. babel/babel-eslint#595

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, thanks!

@@ -347,12 +345,6 @@ actions.deleteNodes = (nodes: any[], plugin: Plugin) => {
plugin,
payload: nodes,
}

if (deleteDescendantsActions) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this as eslint marked it as unreachable. Should deleting descendants still be included?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah — this might be a bad merge. If you look at deleteNode, it finds descendant nodes to delete above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've reverted this, handling it in #5106 instead.

@m-allanson
Copy link
Contributor Author

This is ready for another look.

@KyleAMathews KyleAMathews merged commit bcd24fa into gatsbyjs:v2 Apr 25, 2018
@KyleAMathews
Copy link
Contributor

🙏nice work!

@m-allanson m-allanson deleted the v2-eslint branch April 25, 2018 16:11
mwfrost pushed a commit to mwfrost/gatsby that referenced this pull request Apr 20, 2023
* Fix lint errors

* Revert this, handled in PR 5106

* Pin babel-eslint to a version that supports @babel/plugin-proposal-optional-chaining
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