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

chore(requires-writer): update auto-generated code to not trigger no-mixed-operators eslint warnings/errors #26356

Merged
merged 1 commit into from
Aug 11, 2020

Conversation

pieh
Copy link
Contributor

@pieh pieh commented Aug 11, 2020

Description

Code we generate in (a)sync-requires.js (virtual) modules currently doesn't pass no-mixed-operators eslint rule. This is quick change to make it pass. This is for cases when users might use some custom eslint setups (tho I probably should follow up and see if we have documentation for eslint-loader modifications anywhere and possibly adjust ignore setting there to ignore modules from _this_is_virtual_fs_path_/$virtual (as user might have very speciffic rules that would warn/error on code we autogenerate)

Related Issues

Fixes #26319

@pieh pieh requested a review from a team as a code owner August 11, 2020 09:07
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Aug 11, 2020
@gatsby-cloud
Copy link

gatsby-cloud bot commented Aug 11, 2020

Gatsby Cloud Build Report

🚩 Your build failed. See the build logs here

Errors

Error in "/usr/src/app/www/www/node_modules/gatsby-plugin-page-creator/gatsby-node.js": Cannot find module 'gatsby-cli/lib/reporter'
Require stack:
- /usr/src/app/www/www/node_modules/gatsby-plugin-page-creator/is-valid-collection-path-implementation.js
- /usr/src/app/www/www/node_modules/gatsby-plugin-page-creator/create-pages-from-collection-builder.js
- /usr/src/app/www/www/node_modules/gatsby-plugin-page-creator/create-page-wrapper.js
- /usr/src/app/www/www/node_modules/gatsby-plugin-page-creator/gatsby-node.js
- /usr/src/app/www/www/node_modules/gatsby/dist/bootstrap/resolve-module-exports.js
- /usr/src/app/www/www/node_modules/gatsby/dist/bootstrap/load-plugins/validate.js
- /usr/src/app/www/www/node_modules/gatsby/dist/bootstrap/load-plugins/load.js
- /usr/src/app/www/www/node_modules/gatsby/dist/bootstrap/load-plugins/index.js
- /usr/src/app/www/www/node_modules/gatsby/dist/services/initialize.js
- /usr/src/app/www/www/node_modules/gatsby/dist/services/index.js
- /usr/src/app/www/www/node_modules/gatsby/dist/bootstrap/index.js
- /usr/src/app/www/www/node_modules/gatsby/dist/commands/build.js
- /usr/src/app/www/www/node_modules/gatsby/node_modules/gatsby-cli/lib/create-cli.js
- /usr/src/app/www/www/node_modules/gatsby/node_modules/gatsby-cli/lib/index.js
- /usr/src/app/www/www/node_modules/gatsby/dist/bin/gatsby.js
- /usr/src/app/www/www/node_modules/gatsby/cli.js

@gatsby-cloud-staging
Copy link

Gatsby Cloud Build Report

🚩 Your build failed. See the build logs here

Errors

Error in "/usr/src/app/www/examples/gatsbygram/node_modules/gatsby-plugin-page-creator/gatsby-node.js": Cannot find module 'gatsby-cli/lib/reporter'
Require stack:
- /usr/src/app/www/examples/gatsbygram/node_modules/gatsby-plugin-page-creator/is-valid-collection-path-implementation.js
- /usr/src/app/www/examples/gatsbygram/node_modules/gatsby-plugin-page-creator/create-pages-from-collection-builder.js
- /usr/src/app/www/examples/gatsbygram/node_modules/gatsby-plugin-page-creator/create-page-wrapper.js
- /usr/src/app/www/examples/gatsbygram/node_modules/gatsby-plugin-page-creator/gatsby-node.js
- /usr/src/app/www/examples/gatsbygram/node_modules/gatsby/dist/bootstrap/resolve-module-exports.js
- /usr/src/app/www/examples/gatsbygram/node_modules/gatsby/dist/bootstrap/load-plugins/validate.js
- /usr/src/app/www/examples/gatsbygram/node_modules/gatsby/dist/bootstrap/load-plugins/load.js
- /usr/src/app/www/examples/gatsbygram/node_modules/gatsby/dist/bootstrap/load-plugins/index.js
- /usr/src/app/www/examples/gatsbygram/node_modules/gatsby/dist/services/initialize.js
- /usr/src/app/www/examples/gatsbygram/node_modules/gatsby/dist/services/index.js
- /usr/src/app/www/examples/gatsbygram/node_modules/gatsby/dist/bootstrap/index.js
- /usr/src/app/www/examples/gatsbygram/node_modules/gatsby/dist/commands/build.js
- /usr/src/app/www/examples/gatsbygram/node_modules/gatsby/node_modules/gatsby-cli/lib/create-cli.js
- /usr/src/app/www/examples/gatsbygram/node_modules/gatsby/node_modules/gatsby-cli/lib/index.js
- /usr/src/app/www/examples/gatsbygram/node_modules/gatsby/dist/bin/gatsby.js
- /usr/src/app/www/examples/gatsbygram/node_modules/gatsby/cli.js

@pieh pieh added topic: webpack/babel Webpack or babel type: maintenance An issue or pull request describing a change that isn't a bug, feature or documentation change and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Aug 11, 2020
@@ -195,7 +195,7 @@ export const writeAll = async (state: IGatsbyState): Promise<boolean> => {
let syncRequires = `${hotImport}

// prefer default export if available
const preferDefault = m => m && m.default || m
const preferDefault = m => (m && m.default) || m
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess in the future this will be

Suggested change
const preferDefault = m => (m && m.default) || m
const preferDefault = m => m?.default ?? m

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case I'm not sure which one is more readable :)

But I think we could use your suggestions because this file should be handled by babel so both optional chaining and nullish coalescing operator should work just fine (tho users do have option to use their own custom babel config, so probably keeping old style would result in less potential for problems)

Copy link
Contributor

@wardpeet wardpeet Aug 11, 2020

Choose a reason for hiding this comment

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

Old syntax is less bytes when transpiled

@sidharthachatterjee sidharthachatterjee merged commit 00c8710 into master Aug 11, 2020
@sidharthachatterjee sidharthachatterjee deleted the fix/sync-requires/no-mixed-operators branch August 11, 2020 10:18
@sidharthachatterjee
Copy link
Contributor

Published in gatsby@2.24.41

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: webpack/babel Webpack or babel type: maintenance An issue or pull request describing a change that isn't a bug, feature or documentation change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ESLint warnings in file that doesn't exist
4 participants