Repository navigation
fix: stylelint config for nested installs and Vue style blocks - #100
Conversation
|
I would recommend merging #99 before this so the package upgrades can be done on the correct Node version |
7db5866 to
9b43d13
Compare
yokuze
left a comment
There was a problem hiding this comment.
Also needs rebase to fix merge conflicts.
| ``` | ||
|
|
||
| The `<style>` blocks of `.vue` single-file components are linted with the same rules as | ||
| `.scss` files. A `.vue` file without a `<style>` block is empty input to stylelint, which |
There was a problem hiding this comment.
A style-less .vue file is not empty input, so this sentence gives the wrong reason for --allow-empty-input. Could it say the flag keeps stylelint from failing when the glob matches no files?
Details
With stylelint 16.20.0 and this branch installed in a scratch project:
stylelint './src/**/*.{scss,vue}'exits 0 whensrc/contains only a.vuefile with no<style>block.- The same command exits with
NoFilesFoundError: No files matching the pattern "./src/**/*.{scss,vue}" were found.whensrc/has no.scssor.vuefiles.
Suggested wording:
The `<style>` blocks of `.vue` single-file components are linted with the same rules as
`.scss` files. The example passes `--allow-empty-input` so that the command does not fail
in a project that has no `.scss` or `.vue` files.Comment added by AI model Claude
There was a problem hiding this comment.
Agreed, and confirmed: a .vue file with no <style> block exits 0 without the flag, and only a glob with no matches fails. I used your suggested wording in 3222428.
Comment added by AI model Claude
| }, | ||
| { | ||
| files: [ '**/*.vue' ], | ||
| customSyntax: require.resolve('postcss-html'), |
There was a problem hiding this comment.
This override also applies the SCSS rule set to plain <style> blocks with no lang, so CSS-only checks no longer run there. Is that intended?
Details
declaration-property-value-no-unknown, function-no-unknown, and media-query-no-invalid are off for every <style> block in a .vue file. With this branch, color: rad; inside <style> (no lang) passes, while the same line in a .css file reports Unexpected unknown value "rad" for property "color". On master, the .vue block reported it too.
If all Vue style blocks are expected to use lang="scss", this is fine, and a sentence in the README would make that explicit. Otherwise, postcss-html cannot pick rules per lang, so the trade-off may only need a note.
Comment added by AI model Claude
There was a problem hiding this comment.
Yes, it is intended. Our Vue components use <style lang="scss">, and overrides select rules per file, not per lang, so there is no way to keep the CSS-only checks for a plain block in the same file. In 3222428 the README now says that Vue style blocks must use lang="scss" and names the checks that do not run on a plain <style> block. The comment above SCSS_RULES in stylelint-config.cjs says the same.
Comment added by AI model Claude
Version 3.1.2 imported postcss without declaring it as a dependency, so it relied on npm hoisting stylelint's copy of postcss next to it. When a project upgrades from v2 with an existing package-lock.json, npm nests postcss under stylelint instead, and the plugin fails with "Cannot find package 'postcss'". Version 3.1.3 declares postcss as a dependency and moves stylelint to a peer dependency.
Stylelint imports a `customSyntax` given as a bare package name from its own location in node_modules, not from the config file that names it. When npm nests postcss-scss and postcss-html under @silvermine/standardization instead of hoisting them next to stylelint, linting fails with "Cannot resolve custom syntax module". That is the layout npm produces when a project upgrades from v2 of this package with an existing package-lock.json. Move the configuration into `stylelint-config.cjs`, where `require.resolve()` turns both parsers into absolute paths. `.stylelintrc.yml` now only extends that file, so projects keep extending the same entry point as before.
14b7d95 to
6e293f9
Compare
The `**/*.vue` override only turned off `at-rule-no-unknown`, so the CSS-only rules that the `**/*.scss` override disables (for example `declaration-property-value-no-unknown` and `function-no-unknown`) still ran against the `<style lang="scss">` blocks of Vue components. A SCSS variable in a `transition` value was reported as an unknown value. Both overrides now share the same SCSS rule set, so a Vue style block is linted exactly like a `.scss` file. The stylelint test runner now also picks up `.vue` fixtures. The new pass fixture fails against the previous config, and the fail fixture proves that Vue style blocks are linted at all. Fixes silvermine#97
Vue's `v-bind()` in CSS takes the name of a reactive binding from the component's script, which follows our camelCase naming for TypeScript constants. `value-keyword-case` reported those names as uppercase keywords. Ignore the rule inside `v-bind()` only, as proposed in the issue. Fixes silvermine#94
markdownlint 0.38 added MD059, which rejects link text such as "here" or "this page". Our documentation uses that style in several repositories, so projects would start failing `npm run markdownlint` when they pick up this package. Turn the rule off in the shared config. Proposed in silvermine#96.
The repo's .editorconfig asks for 3-space indentation in every file, but package.json still used the 2 spaces that `npm init` wrote years ago. npm keeps whatever indentation it finds in package.json, and it formats package-lock.json to match, so re-indent both now. Whitespace-only change. Proposed in silvermine#96.
6e293f9 to
3af3611
Compare
Why
Projects that upgrade from v2 of this package to v3.0.0 with an existing
package-lock.jsonend up withpostcss,postcss-htmlandpostcss-scssnested under@silvermine/standardizationorstylelintand no copy at the top level ofnode_modules(npm keeps the old v2 packages in the top-level slots while it builds the new tree, then prunes them). In that layoutnpm run stylelintfails before linting a single file:@stylistic/stylelint-plugin@3.1.2importspostcsswithout declaring it, so it fails withCannot find package 'postcss'.customSyntaxgiven as a bare package name from its own location, sopostcss-scssandpostcss-htmlfail withCannot resolve custom syntax module.I hit both while upgrading a consumer project from 2.2.3 to 3.0.0. Projects already on 3.0.0 work only when their lockfiles happen to hoist those packages. On top of that, the
**/*.vueoverride did not turn off the CSS-only rules that the**/*.scssoverride turns off (#97), andv-bind()values trippedvalue-keyword-case(#94).What
One commit per change, each passing
npm testandnpm run standardson its own:build: upgrade @stylistic/stylelint-plugin to 3.1.3– 3.1.3 declarespostcssas a dependency and movesstylelintto a peer dependency (bugfix-only release). The lockfile diff is kept to the plugin's own entry on purpose, see "Lockfile note" below.fix: resolve stylelint custom syntaxes by absolute path– the configuration moves intostylelint-config.cjs, whererequire.resolve()turns both parsers into absolute paths..stylelintrc.ymlnow only extends that file, so consumers keep extending the same entry point. README tells consumers not to setcustomSyntaxthemselves.fix: apply the SCSS rule overrides to Vue style blocks (#97)– both overrides share one SCSS rule set. The test runner now also runs.vuefixtures; the new pass fixture fails against the previous config.fix: allow any case inside v-bind() in Vue style values (#94)–value-keyword-caseignoresv-bind(), with a fixture.fix: disable markdownlint descriptive-link-text rule– ported from Update markdownlint and stylelint to the latest #96. markdownlint 0.38 (what our pinned cli2 ships) enables MD059, and our repos' docs use link text such as "here" in several places.style: indent package.json and its lockfile with 3 spaces– ported from Update markdownlint and stylelint to the latest #96, whitespace only. Matches.editorconfig; npm keeps the indentation it finds and formats the lockfile to match.Fixes #97
Fixes #94
Verification
npm testandnpm run standardspass locally and in CI..stylelintrc.ymlthat most of our repos use, with--config-basedir, and through both the CommonJS and ESM Node APIs (what webpack and Grunt plugins call).Why
.cjsand not.mjsThe format of the shared config is invisible to consumers: they extend the yml and stylelint loads whatever it points at. An
.mjsversion of the same file (usingcreateRequire(import.meta.url)) behaves identically in the same tests, and the.cjsextension is explicitly CommonJS, so ESM consumers ("type": "module") are unaffected either way.I chose
.cjsfor consistency with this repo today: the release scripts, bin, commitlint config and markdownlint config are all CommonJS, and ESLint 6 here lints CommonJS. Our newest repos are ESM and stylelint recommends ESM configs, so the natural follow-up is a repo-wide move to ESM, at which point this file becomesstylelint-config.mjswith a three-line change and consumers notice nothing.Relation to open PR #96
#96 moves to stylelint 17 and conflicts with master. This PR stays on stylelint 16.20.0, so it can land first. Two of its smaller changes are ported here (commits 5 and 6). Not ported, on purpose:
@stylistic5.x required).nesting-selector-no-missing-scoping-rootwithignoreAtRules: [mixin]. The rule only exists from stylelint 16.23 and config-recommended enables it from 17.0.0, so it belongs with that bump. Our SCSS uses&inside@mixinblocks a lot, so theignoreAtRulespart will be needed then.gitignoreoption, is already in use since 3.0.0 with cli2 0.18.1.Future work
.mjsas part of it.@stylistic/stylelint-plugin5.x and the nesting-selector rule configured as Update markdownlint and stylelint to the latest #96 proposes.Lockfile note
npm installin this repo (with the pinned npm 11.6.1 as well as newer npm) drops the lock entrynode_modules/stylelint/node_modules/typescript, an optional peer of cosmiconfig. CI'snpm cithen rejects the lock as out of sync and lists missing@typescript/typescript-*packages. The lock here keeps that entry and changes only what the plugin bump needs;npx npm@11.6.1 cipasses on it.Notes for consumers
customSyntax: postcss-htmlitself (some do, from the v2 days when the shared config had no Vue override) still fails in the nested layout. Drop that line; the shared config covers.vuefiles.npm ciafter bumping. A stalenode_modulesthat still holds v2'spostcss-scssmakes stylelint 16 crash withCannot read properties of undefined (reading 'nodes'), becausepostcss-htmllooks for language parsers in the working directory first.