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 custom syntax require error handling #5635

Merged
merged 1 commit into from Oct 25, 2021

Conversation

ybiquitous
Copy link
Member

Which issue, if any, is this issue related to?

Is there anything in the PR that needs further explanation?

Node.js require() may cause other errors than MODULE_NOT_FOUND. This change aims to throw such errors.
See also: https://nodejs.org/api/errors.html#errors_module_not_found

@jeddy3 jeddy3 changed the title Better error handling for MODULE_NOT_FOUND Fix custom syntax require error handling Oct 25, 2021
Copy link
Member

@jeddy3 jeddy3 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@jeddy3 jeddy3 merged commit 71e0d50 into main Oct 25, 2021
@jeddy3 jeddy3 deleted the better-error-handling-for-module-not-found branch October 25, 2021 15:33
@jeddy3
Copy link
Member

jeddy3 commented Oct 25, 2021

  • Fixed: custom syntax require error handling (#5635).

@XhmikosR
Copy link
Member

Just wondering, couldn't we start using the optional chaining operator in cases like this? Like error?.code === 'MODULE_NOT_FOUND'

@jeddy3
Copy link
Member

jeddy3 commented Oct 25, 2021

I don't think optional chaining is available in node 12.20. (see node green and mdn). If it is, though, we can adopt it.

@XhmikosR
Copy link
Member

Ah, true, I guess one thing to do when Node.js 12 support is dropped :)

@cschwebk
Copy link

@jeddy3 A possible improvement to this error message would be to include the full error message and require stack in the output, instead of assuming that the module that cannot be found is the value in customSyntax. I ran into this same error message today due to an unfulfilled peerDependency (postcss-syntax is a peerDependency of @stylelint/postcss-css-in-js).

$ stylelint src/client/**/*.{js,scss}
Error: Cannot find module 'postcss-syntax/load-syntax'
Require stack:
- /Users/cschwebk/project/node_modules/@stylelint/postcss-css-in-js/extract.js
- /Users/cschwebk/project/node_modules/@stylelint/postcss-css-in-js/index.js
- /Users/cschwebk/project/stylelint.config.js
- /Users/cschwebk/project/node_modules/stylelint/node_modules/cosmiconfig/dist/loaders.js
- /Users/cschwebk/project/node_modules/stylelint/node_modules/cosmiconfig/dist/ExplorerBase.js
- /Users/cschwebk/project/node_modules/stylelint/node_modules/cosmiconfig/dist/Explorer.js
- /Users/cschwebk/project/node_modules/stylelint/node_modules/cosmiconfig/dist/index.js
- /Users/cschwebk/project/node_modules/stylelint/lib/getConfigForFile.js
- /Users/cschwebk/project/node_modules/stylelint/lib/createStylelint.js
- /Users/cschwebk/project/node_modules/stylelint/lib/printConfig.js
- /Users/cschwebk/project/node_modules/stylelint/lib/cli.js
- /Users/cschwebk/project/node_modules/stylelint/bin/stylelint.js
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:889:15)
    at Function.Module._load (internal/modules/cjs/loader.js:745:27)
    at Module.require (internal/modules/cjs/loader.js:961:19)
    at require (/Users/cschwebk/project/node_modules/v8-compile-cache/v8-compile-cache.js:159:20)
    at Object.<anonymous> (/Users/cschwebk/project/node_modules/@stylelint/postcss-css-in-js/extract.js:4:20)
    at Module._compile (/Users/cschwebk/project/node_modules/v8-compile-cache/v8-compile-cache.js:192:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1101:10)
    at Module.load (internal/modules/cjs/loader.js:937:32)
    at Function.Module._load (internal/modules/cjs/loader.js:778:12)
    at Module.require (internal/modules/cjs/loader.js:961:19)

@jeddy3
Copy link
Member

jeddy3 commented Oct 27, 2021

improvement to this error message would be to include the full error message and require stack in the output, instead of assuming that the module that cannot be found is the value in customSyntax.

Sounds good. Pull request welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants