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 module path not properly resolving #5632

Closed
niksy opened this issue Oct 22, 2021 · 22 comments · Fixed by #5728
Closed

Fix custom syntax module path not properly resolving #5632

niksy opened this issue Oct 22, 2021 · 22 comments · Fixed by #5728
Labels
priority: high is impactful on users status: ready to implement is ready to be worked on by someone type: bug a problem with a feature or rule

Comments

@niksy
Copy link

niksy commented Oct 22, 2021

Clearly describe the bug

As per stylelint-scss/stylelint-config-standard-scss#2, custom syntax module path is not properly resolved.

Stylelint tries to resolve custom syntax location (stylelint/stylelint@main/lib/getPostcssResult.js#L95) but uses Node require algorithm, so it search from that file upwards.

If you have dependancy tree where postcss-scss is not in one of the root directories, but is nested inside this config’s node_modules (it happens if you have different versions of same package), path resolution fails.

Which rule, if any, is the bug related to?

None, it’s related to core functionality

What code is needed to reproduce the bug?

Even empty SCSS file would do.

What Stylelint configuration is needed to reproduce the bug?

{
  "extends": "stylelint-config-standard-scss"
}

But postcss-scss shouldn’t be installed inside root node_modules.

Which version of Stylelint are you using?

14.0.0

How are you running stylelint: CLI, PostCSS plugin, Node.js API?

stylelint "**/*.scss"

Does the bug relate to non-standard syntax (e.g. SCSS, Less etc.)?

Yes, it's related to SCSS custom syntax.

What did you expect to happen?

Should return at least error/warning related to style code or exit code 0 if everything is OK.

What actually happened (e.g. what warnings or errors did you get)?

Cannot resolve custom syntax module "postcss-scss". Check that module "postcss-scss" is available and spelled correctly.

at getCustomSyntax (node_modules/stylelint/lib/getPostcssResult.js:97:10)
at getPostcssResult (node_modules/stylelint/lib/getPostcssResult.js:41:5)
at lintSource (node_modules/stylelint/lib/lintSource.js:76:20)
at Function.standalone [as lint] (node_modules/stylelint/lib/standalone.js:132:26)
@jeddy3
Copy link
Member

jeddy3 commented Oct 26, 2021

This is likely caused by an incompatible version of PostCSS in the tree. (The error will be clearer in the next release). You can use resolutions to force the use of PostCSS 8 if all your dependencies don't specify it:

Root package.json:

"resolutions": {
  "postcss": "^8.3.11"
},

@jeddy3 jeddy3 closed this as completed Oct 26, 2021
@afdev82
Copy link

afdev82 commented Oct 26, 2021

Thank you for the explanation, but I found that for me the only reason for the wrong version of post-scss is because the stylelint-config-recommended-scss package depends on it:

yarn why postcss-scss
yarn why v1.22.15
[1/4] Why do we have the module "postcss-scss"...?
[2/4] Initialising dependency graph...
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "postcss-scss@4.0.1"
info Reasons this module exists
   - "stylelint-config-standard-scss#stylelint-config-recommended-scss" depends on it
   - Hoisted from "stylelint-config-standard-scss#stylelint-config-recommended-scss#postcss-scss"
info Disk size without dependencies: "64KB"
info Disk size with unique dependencies: "64KB"
info Disk size with transitive dependencies: "64KB"
info Number of shared dependencies: 0
Done in 0.61s.

I think than the stylelint-config-recommended-scss should be updated (requiring post-scss v8) in order to work with stylelint v14 or am I missing something here?
Using resolutions is only a temporary solution.

@jeddy3
Copy link
Member

jeddy3 commented Oct 26, 2021

It's incompatible versions of postcss, not postcss-scss. You'll need to use yarn why postcss to find them.

@fwextensions
Copy link

fwextensions commented Oct 31, 2021

This doesn't seem related only to an incompatible version of postcss. An older version of css-loader was including postcss@7.x, but after installing postcss at the top level, I still saw the error:

+-- css-loader@2.1.1
| +-- icss-utils@4.1.1
| | `-- postcss@7.0.39
| +-- postcss-modules-extract-imports@2.0.0
| | `-- postcss@7.0.39
| +-- postcss-modules-local-by-default@2.0.6
| | `-- postcss@7.0.39
| +-- postcss-modules-scope@2.2.0
| | `-- postcss@7.0.39
| +-- postcss-modules-values@2.0.0
| | `-- postcss@7.0.39
| `-- postcss@7.0.39
+-- postcss@8.3.11
+-- stylelint-config-standard-scss@2.0.0
| `-- stylelint-config-recommended-scss@5.0.0
|   `-- postcss-scss@4.0.2
|     `-- postcss@8.3.11 deduped
`-- stylelint@14.0.1
  +-- postcss-safe-parser@6.0.0
  | `-- postcss@8.3.11 deduped
  `-- postcss@8.3.11 deduped

Updating css-loader to 5.2.7, which used the latest postcss, didn't help either. I also had to install postcss-scss at the top level:

+-- postcss-scss@4.0.2
`-- stylelint-config-standard-scss@2.0.0
  `-- stylelint-config-recommended-scss@5.0.0
    `-- postcss-scss@4.0.2 deduped

Only then would stylelint work without throwing the Cannot resolve custom syntax module "postcss-scss". error. Surely manually installing these postcss* modules in addition to the stylelint* modules (which require them anyway) shouldn't be necessary?

@jeddy3
Copy link
Member

jeddy3 commented Oct 31, 2021

After updating css-loader, what did npm ls postcss show?

@fwextensions
Copy link

After updating css-loader, it looked like this:

treeme@0.0.1 C:\Projects\c4sf\waterthetrees\wtt_front
+-- css-loader@5.2.7
| +-- icss-utils@5.1.0
| | `-- postcss@8.3.11 deduped
| +-- postcss-modules-extract-imports@3.0.0
| | `-- postcss@8.3.11 deduped
| +-- postcss-modules-local-by-default@4.0.0
| | `-- postcss@8.3.11 deduped
| +-- postcss-modules-scope@3.0.0
| | `-- postcss@8.3.11 deduped
| +-- postcss-modules-values@4.0.0
| | `-- postcss@8.3.11 deduped
| `-- postcss@8.3.11 deduped
+-- postcss-scss@4.0.2
| `-- postcss@8.3.11 deduped
+-- postcss@8.3.11
`-- stylelint@14.0.1
  +-- postcss-safe-parser@6.0.0
  | `-- postcss@8.3.11 deduped
  `-- postcss@8.3.11 deduped

So all the uses were pointing at the latest 8.3.11 version, AFAICT, but that wasn't sufficient to fix the error.

@jeddy3
Copy link
Member

jeddy3 commented Nov 1, 2021

Thanks. Let's reopen this as there seems to be more going on than incompatible versions.

Does anyone have any thoughts, e.g. is #5633 the answer here?

@jeddy3 jeddy3 reopened this Nov 1, 2021
@jeddy3 jeddy3 added status: needs investigation triage needs further investigation and removed status: needs discussion triage needs further discussion labels Nov 1, 2021
@ybiquitous
Copy link
Member

@fwextensions Is it possible to share your reproduction code to raise the error?

@niksy
Copy link
Author

niksy commented Nov 2, 2021

Thanks. Let's reopen this as there seems to be more going on than incompatible versions.

Does anyone have any thoughts, e.g. is #5633 the answer here?

My thinking is that this should be the answer since it follows same logic for resolving 3rd-party dependencies as for presets and plugins. If it doesn’t work there, Stylelint maybe has some other issues with resolving algorithm.

So, maybe we should reopen that PR?

@ybiquitous
Copy link
Member

FYI. Here are code resolving customSyntax:

stylelint/lib/cli.js

Lines 360 to 362 in f218b59

if (cli.flags.customSyntax) {
optionsBase.customSyntax = getModulePath(process.cwd(), cli.flags.customSyntax);
}

const syntax = options.customSyntax
? getCustomSyntax(options.customSyntax)
: cssSyntax(stylelint, options.filePath);

function getCustomSyntax(customSyntax) {
let resolved;
if (typeof customSyntax === 'string') {
try {
resolved = require(customSyntax);
} catch (error) {
// @ts-expect-error -- TS2571: Object is of type 'unknown'.
if (error && typeof error === 'object' && error.code === 'MODULE_NOT_FOUND') {
throw new Error(
`Cannot resolve custom syntax module "${customSyntax}". Check that module "${customSyntax}" is available and spelled correctly.`,
);
}
throw error;
}
/*
* PostCSS allows for syntaxes that only contain a parser, however,
* it then expects the syntax to be set as the `parse` option.
*/
if (!resolved.parse) {
resolved = {
parse: resolved,
stringify: postcss.stringify,
};
}
return resolved;
}
if (typeof customSyntax === 'object') {
if (typeof customSyntax.parse === 'function') {
resolved = { ...customSyntax };
} else {
throw new TypeError(
`An object provided to the "customSyntax" option must have a "parse" property. Ensure the "parse" property exists and its value is a function.`,
);
}
return resolved;
}
throw new Error(`Custom syntax must be a string or a Syntax object`);
}

@niksy
Copy link
Author

niksy commented Nov 2, 2021

@ybiquitous as mentioned in my original post, problem is probably in using just the standard Node require. There’s a reason why presets and plugins implement various levels of resolving, one of it probably to avoid longstanding resolution "bug" present in ESLint.

@ybiquitous
Copy link
Member

@niksy Thank you for providing more information. It seems better to use our getModulePath() utility to resolve customSyntax, instead of Node.js require().

@jeddy3 jeddy3 added status: ready to implement is ready to be worked on by someone type: bug a problem with a feature or rule and removed status: needs investigation triage needs further investigation labels Nov 2, 2021
@jeddy3 jeddy3 changed the title Custom syntax module path not properly resolved Fix custom syntax module path not properly resolving Nov 2, 2021
@fwextensions
Copy link

@ybiquitous the code is in a private repo that somebody else built. A couple of scripts in its package.json referred to stylelint, but that package wasn't installed at all. I figured I could make it work by just adding that package and stylelint-config-standard-scss, based on the docs. But that's when I ran into this issue. I've never used stylelint before, so I'm not sure how helpful I can be.

One thing to note is that this project has only .scss files in it. The script for linting looks like this: "lint:css": "stylelint \"client/src/**/*.scss\"",.

@ybiquitous
Copy link
Member

@fwextensions Thanks for the response. I'd like to get a small reproduction code if possible... 😅

@monsieurnebo
Copy link

Same issue over here while updating to stylelint v14 💥

@justafish
Copy link

I solved this in Yarn 2+ by adding postcss and postcss-scss as direct devDependencies to my project, and adding this to package.json:

"resolutions": {
    "postcss": "^8"
  }

@jeddy3 jeddy3 added the priority: high is impactful on users label Nov 15, 2021
@ybiquitous
Copy link
Member

ybiquitous commented Nov 17, 2021

I got a small reproduction as below. Now I am looking into the error.

package.json:

{
  "dependencies": {
    "postcss": "7.0.39",
    "stylelint": "14.1.0",
    "stylelint-config-recommended-scss": "5.0.1"
  },
  "scripts": {
    "lint": "stylelint *.scss"
  },
  "stylelint": {
    "extends": "stylelint-config-recommended-scss"
  }
}

a.scss:

a {}

Run:

$ node -v ; npm -v
v16.13.0
8.1.3

$ npm i
...

$ npm run lint

> lint
> stylelint *.scss

Error: Cannot resolve custom syntax module "postcss-scss". Check that module "postcss-scss" is available and spelled correctly.
    at getCustomSyntax (/Users/koba/tmp/foo/node_modules/stylelint/lib/getPostcssResult.js:99:11)
    at getPostcssResult (/Users/koba/tmp/foo/node_modules/stylelint/lib/getPostcssResult.js:41:5)
    at lintSource (/Users/koba/tmp/foo/node_modules/stylelint/lib/lintSource.js:76:20)
    at async /Users/koba/tmp/foo/node_modules/stylelint/lib/standalone.js:218:27
    at async Promise.all (index 0)
    at async standalone (/Users/koba/tmp/foo/node_modules/stylelint/lib/standalone.js:257:22)
$ npm ls postcss
foo@ /Users/koba/tmp/foo
├── postcss@7.0.39
├─┬ stylelint-config-recommended-scss@5.0.1
│ └─┬ postcss-scss@4.0.2
│   └── postcss@8.3.11
└─┬ stylelint@14.1.0
  ├─┬ postcss-safe-parser@6.0.0
  │ └── postcss@8.3.11 deduped
  └── postcss@8.3.11

EDIT: For coexistence of different versions of PostCSS, postcss-scss is installed in a deeper place than node_modules/. See below:

$ find node_modules -type d -name postcss-scss
node_modules/stylelint-config-recommended-scss/node_modules/postcss-scss
$ node -pe "require('postcss-scss')"
node:internal/modules/cjs/loader:936
  throw err;
  ^

Error: Cannot find module 'postcss-scss'
Require stack:
- /Users/koba/tmp/foo/[eval]
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:933:15)
    at Function.Module._load (node:internal/modules/cjs/loader:778:27)
    at Module.require (node:internal/modules/cjs/loader:1005:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at [eval]:1:1
    at Script.runInThisContext (node:vm:129:12)
    at Object.runInThisContext (node:vm:305:38)
    at node:internal/process/execution:81:19
    at [eval]-wrapper:6:22
    at evalScript (node:internal/process/execution:80:60) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [ '/Users/koba/tmp/foo/[eval]' ]
}
$ node -pe "require('stylelint-config-recommended-scss/node_modules/postcss-scss')"
{ parse: [Function: scssParse], stringify: [Function: scssStringify] }

@tom-ricci
Copy link

tom-ricci commented Nov 17, 2021

Hey, I'm trying to use stylelint with CRA and am having the same issue. I'm fine installing postcss-scss, but the only problem is CRA uses postcss 7, and postcss-scss needs v8. Is there a way to get this working with postcss 7? (or if not, would simply updating to postcss 8 in my react project work and not have any breaking changes?)

edit: i know i could switch to cra 5, but id rather stick with 4 as 5 is still an alpha release

@ybiquitous
Copy link
Member

As I investigated on #5632 (comment), the Node.js require() function cannot resolve postcss-scss in a deeper place when there are different versions of PostCSS under the node_modules/ directory.

This seems a temporary problem until the PostCSS community finishes migrating to PostCSS 8, so I think now there are the following options:

  1. Promote each sharable config to use the customSyntax: require('...') pattern, instead of customSyntax: '...'. For example of stylelint-config-recommended-scss:
    - customSyntax: 'postcss-scss',
    + customSyntax: require('postcss-scss'),
    • This way is straightforward.
    • The community needs to rewrite and republish each sharable config.
  2. Implement our own custom resolution logic (or, any other package?) in the core stylelint package.

What do you think?

@tom-ricci
Copy link

Even though I'm just a user, I'd say the first option is the best for this case. Since it's a temporary issue, might as well take the easier approach. There's not that many people that are affected by this, and anyone writing/using a shareable config that does need to do this could easily fix it given there's only 1 line to change.

@tom-ricci
Copy link

Welp seems like I'm not just a user anymore lol

@jeddy3
Copy link
Member

jeddy3 commented Nov 21, 2021

@ybiquitous Thank you very much for the investigation! I agree that the first approach is best. I've opened the pull request to update our docs.

@tom-ricci Thanks for opening the issue and pull request in the scss package. Even though I'm not usually active in that repo, I've merged and released as it's related to the 14.0.0 release and seems to be impacting a number of users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: high is impactful on users status: ready to implement is ready to be worked on by someone type: bug a problem with a feature or rule
8 participants