Skip to content

/ gatsby Public

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

Jump to bottom

chore: update eslint to fix linting issues #29988

Merged
merged 10 commits into from Mar 4, 2021
Merged

chore: update eslint to fix linting issues #29988

merged 10 commits into from Mar 4, 2021
+860 −892

Conversation

@wardpeet
Copy link
Member

@wardpeet wardpeet commented Mar 4, 2021

Description

Upgraded eslint to latest and fixed rules along the way.

Documentation

Related Issues
@wardpeet
chore: update eslint to fix linting issues
b577679
wardpeet
wardpeet reviewed Mar 4, 2021
View changes
.babelrc.js
// if (process.env.NODE_ENV !== `test`) {
// ignore.push(`**/__tests__`)
// }
Copy link
Member Author

@wardpeet wardpeet Mar 4, 2021

This breaks linting as babel will return an empty ast. Packages should be build with babel src --out-dir . --ignore \"**/__tests__\"

wardpeet
wardpeet reviewed Mar 4, 2021
View changes
.eslintrc.js
"prettier/flowtype",
"prettier/react",
Copy link
Member Author

@wardpeet wardpeet Mar 4, 2021

these are now inside prettier

.eslintrc.js
],
plugins: ["flowtype", "prettier", "react", "filenames"],
plugins: [`flowtype`, `prettier`, `react`, `filenames`, `@babel`],
Copy link
Member Author

@wardpeet wardpeet Mar 4, 2021

Added @babel to augment the this rules

.eslintrc.js
"prefer-promise-reject-errors": `warn`,
"no-prototype-builtins": `warn`,
"guard-for-in": `warn`,
Copy link
Member Author

@wardpeet wardpeet Mar 4, 2021

These are new rules and moved them to warnings because there were too many.

.eslintrc.js Outdated
"prefer-promise-reject-errors": `warn`,
"no-prototype-builtins": `warn`,
"guard-for-in": `warn`,
"spaced-comment": [`error`, `always`, { markers: [`/`] }],
Copy link
Member Author

@wardpeet wardpeet Mar 4, 2021

enable typescript /// referrence comments

.eslintrc.js
camelcase: [
`error`,
{
properties: `never`,
ignoreDestructuring: true,
allow: [`^unstable_`],
},
],
Copy link
Member Author

@wardpeet wardpeet Mar 4, 2021

I have no idea why this wasn't necessary before 🤷

packages/gatsby-plugin-netlify-cms/src/cms-identity.js
@@ -1,4 +1,4 @@
/* global __PATH_PREFIX__ CMS_PUBLIC_PATH */
/* global CMS_PUBLIC_PATH */
Copy link
Member Author

@wardpeet wardpeet Mar 4, 2021

PATH_PREFIX is in the global globals list

packages/gatsby-remark-images-contentful/src/index.js
if (node.url.indexOf(`images.ctfassets.net`) === -1) {
return resolve()
}
Copy link
Member Author

@wardpeet wardpeet Mar 4, 2021

This is really weird and I'm astonished that this hasn't lead to any bugs.

packages/gatsby/src/commands/develop.ts
@@ -71,12 +71,12 @@ const doesConfigChangeRequireRestart = (
const getDebugPort = (port?: number): number => port ?? 9229

export const getDebugInfo = (program: IProgram): IDebugInfo | null => {
if (program.hasOwnProperty(`inspect`)) {
if (Object.prototype.hasOwnProperty.call(program, `inspect`)) {
Copy link
Member Author

@wardpeet wardpeet Mar 4, 2021

I was fixing this rule but eventually moved to warning.

Copy link
Contributor

@ascorbic ascorbic Mar 4, 2021

Can you explain why this is needed?

Copy link
Member Author

@wardpeet wardpeet Mar 4, 2021

https://eslint.org/docs/rules/no-prototype-builtins

It's a security risk as technically program could have overwritten hasOwnProperty

Copy link
Contributor

@ascorbic ascorbic Mar 4, 2021

Neat. I'd not thought of that

wardpeet added 2 commits Mar 4, 2021
@wardpeet
cleanup
c2b86db
@wardpeet
remove /
2ae565e
ascorbic
ascorbic reviewed Mar 4, 2021
View changes
packages/gatsby/src/utils/webpack-utils.ts
@@ -784,7 +784,7 @@ export const createWebpackUtils = (
],
...eslintConfig(schema, jsxRuntimeExists),
}
//@ts-ignore
// @ts-ignore
Copy link
Contributor

@ascorbic ascorbic Mar 4, 2021

Now I'm wondering why our ts rules are allowing this!

Copy link
Member Author

@wardpeet wardpeet Mar 4, 2021

Do we have a lint rule for the space in typescript?

Copy link
Contributor

@ascorbic ascorbic Mar 4, 2021

No, but we do have a rule that requires a description when there's a ts-ignore comment

Copy link
Member Author

@wardpeet wardpeet Mar 4, 2021

It's only a warning :)

        "@typescript-eslint/ban-ts-comment": [
          `warn`,
          { "ts-ignore": `allow-with-description` },
        ],

@wardpeet @ascorbic
Update packages/gatsby/src/commands/serve.ts
240244f 
Co-authored-by: Matt Kane <matt@gatsbyjs.com>
@wardpeet wardpeet marked this pull request as ready for review Mar 4, 2021
@wardpeet
update config
1f510d2
@wardpeet
update circle to keep babel-preset-gatsby-package so we can use it in
f28eb27 
our babel config
wardpeet added 4 commits Mar 4, 2021
@wardpeet
fix unit tests
6ad12ef
@wardpeet
fix snapshot
2bf85ba
@wardpeet
fix thigns
d2b5807
@wardpeet
update gatsby-node
76e1713
vladar
vladar approved these changes Mar 4, 2021
View changes
@wardpeet wardpeet merged commit 5636389 into master Mar 4, 2021
28 of 31 checks passed
28 of 31 checks passed
@circleci-checks[bot]
build-test Workflow: build-test
Details
ci/circleci: e2e_tests_pnp Your tests failed on CircleCI
Details
ci/circleci: starters_validate Your tests failed on CircleCI
Details
@gatsbybot
Cloud Tests
Details
@gatsbot[bot]
Danger All good
Details
@gatsbot[bot]
Peril All green. Well done.
Details
ci/circleci: bootstrap Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_development_runtime Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_gatsby-image Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_gatsby-static-image Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_path-prefix Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_production_runtime Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_visual-regression Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_artifacts Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_cache_resilience Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_gatsby_cli Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_gatsby_pipeline Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_gatsby_source_wordpress Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_images Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_long_term_caching Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_ssr Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_structured_logging Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: mdx_e2e_tests Your tests passed on CircleCI!
Details
ci/circleci: themes_e2e_tests_development_runtime Your tests passed on CircleCI!
Details
ci/circleci: themes_e2e_tests_production_runtime Your tests passed on CircleCI!
Details
ci/circleci: typecheck Your tests passed on CircleCI!
Details
ci/circleci: unit_tests_node12 Your tests passed on CircleCI!
Details
ci/circleci: unit_tests_node14 Your tests passed on CircleCI!
Details
ci/circleci: windows_unit_tests Your tests passed on CircleCI!
Details
@cypress[bot]
cypress: default-group 160 tests passed in 09:20
Details
@wardpeet wardpeet deleted the fix/eslint branch Mar 4, 2021
@vladar vladar mentioned this pull request Mar 10, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vladar vladar

@ascorbic ascorbic

Assignees
No one assigned
Labels
type: maintenance
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
@wardpeet @vladar @ascorbic