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 cache refresh when config is changed #6356

Merged
merged 15 commits into from Sep 27, 2022
Merged
5 changes: 5 additions & 0 deletions .changeset/happy-oranges-invite.md
@@ -0,0 +1,5 @@
---
"stylelint": patch
---

Fixed: cache refresh when config is changed
73 changes: 73 additions & 0 deletions lib/__tests__/cli.test.js
Expand Up @@ -3,6 +3,7 @@
'use strict';

const path = require('path');
const { promises: fs, existsSync } = require('fs');
const stripAnsi = require('strip-ansi');

const cli = require('../cli');
Expand All @@ -12,6 +13,12 @@ const replaceBackslashes = require('../testUtils/replaceBackslashes');
const fixturesPath = (...elems) => replaceBackslashes(path.join(__dirname, 'fixtures', ...elems));
const { buildCLI } = cli;

const removeFile = async (filePath) => {
if (existsSync(filePath)) {
await fs.unlink(filePath);
}
};
ybiquitous marked this conversation as resolved.
Show resolved Hide resolved

jest.mock('../utils/getStdin', () => () => Promise.resolve(''));

describe('buildCLI', () => {
Expand Down Expand Up @@ -369,4 +376,70 @@ describe('CLI', () => {

expect(output).toBe('Custom formatter reports 1 warning(s).');
});

it('--cache same warning config', async () => {
await cli([
'--cache',
'--cache-location',
fixturesPath('.stylelintcache'),
'--config',
fixturesPath('default-severity-warning.json'),
fixturesPath('empty-block.css'),
]);
await cli([
'--cache',
'--cache-location',
fixturesPath('.stylelintcache'),
'--config',
fixturesPath('default-severity-warning.json'),
fixturesPath('empty-block.css'),
]);
expect(process.stdout.write).toHaveBeenCalledTimes(1);

await removeFile(fixturesPath('.stylelintcache'));
});

it('--cache different error config', async () => {
await cli([
'--cache',
'--cache-location',
fixturesPath('.stylelintcache'),
'--config',
fixturesPath('default-severity-warning.json'),
fixturesPath('empty-block.css'),
]);
await cli([
'--cache',
'--cache-location',
fixturesPath('.stylelintcache'),
'--config',
fixturesPath('config-block-no-empty.json'),
fixturesPath('empty-block.css'),
]);
expect(process.stdout.write).toHaveBeenCalledTimes(2);

await removeFile(fixturesPath('.stylelintcache'));
});

it('--cache same error config', async () => {
await cli([
'--cache',
'--cache-location',
fixturesPath('.stylelintcache'),
'--config',
fixturesPath('config-block-no-empty.json'),
fixturesPath('empty-block.css'),
]);
await cli([
'--cache',
'--cache-location',
fixturesPath('.stylelintcache'),
'--config',
fixturesPath('config-block-no-empty.json'),
fixturesPath('empty-block.css'),
]);
expect(process.stdout.write).toHaveBeenCalledTimes(2);

await removeFile(fixturesPath('.stylelintcache'));
});
ybiquitous marked this conversation as resolved.
Show resolved Hide resolved
});
16 changes: 10 additions & 6 deletions lib/__tests__/standalone-cache.test.js
Expand Up @@ -16,6 +16,10 @@ const removeFile = async (filePath) => {
}
};

const isChanged = (file, targetFilePath) => {
return file.source === targetFilePath && !file.ignored;
};

const cwd = process.cwd();
const fixturesPath = path.join(__dirname, 'fixtures');
const invalidFile = path.join(fixturesPath, 'empty-block.css');
Expand Down Expand Up @@ -74,8 +78,8 @@ describe('standalone cache', () => {
const { results } = await standalone(getConfig());

// Ensure only changed files are linted
expect(results.some((file) => file.source === validFile)).toBe(false);
expect(results.some((file) => file.source === newFileDest)).toBe(true);
expect(results.some((file) => isChanged(file, validFile))).toBe(false);
expect(results.some((file) => isChanged(file, newFileDest))).toBe(true);

const { cache } = fCache.createFromFile(expectedCacheFilePath);

Expand All @@ -98,8 +102,8 @@ describe('standalone cache', () => {
);

// Ensure all files are re-linted
expect(results.some((file) => file.source === validFile)).toBe(true);
expect(results.some((file) => file.source === newFileDest)).toBe(true);
expect(results.some((file) => isChanged(file, validFile))).toBe(true);
expect(results.some((file) => isChanged(file, newFileDest))).toBe(true);
});

it('invalid files are not cached', async () => {
Expand All @@ -111,8 +115,8 @@ describe('standalone cache', () => {
expect(errored).toBe(true);

// Ensure only changed files are linted
expect(results.some((file) => file.source === validFile)).toBe(false);
expect(results.some((file) => file.source === newFileDest)).toBe(true);
expect(results.some((file) => isChanged(file, validFile))).toBe(false);
expect(results.some((file) => isChanged(file, newFileDest))).toBe(true);

const { cache } = fCache.createFromFile(expectedCacheFilePath);

ybiquitous marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
20 changes: 20 additions & 0 deletions lib/lintSource.js
Expand Up @@ -2,6 +2,8 @@

const isPathNotFoundError = require('./utils/isPathNotFoundError');
const lintPostcssResult = require('./lintPostcssResult');
const pkg = require('../package.json');
const hash = require('./utils/hash');
const path = require('path');

/** @typedef {import('stylelint').InternalApi} StylelintInternalApi */
Expand Down Expand Up @@ -65,13 +67,30 @@ module.exports = async function lintSource(stylelint, options = {}) {

const config = configForFile.config;
const existingPostcssResult = options.existingPostcssResult;
const fileCache = options.fileCache;

if (options.cache && fileCache) {
const stylelintVersion = pkg.version;
const hashOfConfig = hash(`${stylelintVersion}_${JSON.stringify(config || {})}`);
ybiquitous marked this conversation as resolved.
Show resolved Hide resolved

fileCache.hashOfConfig = hashOfConfig;

if (options.filePath && !fileCache.hasFileChanged(options.filePath)) {
return options.existingPostcssResult
? Object.assign(options.existingPostcssResult, {
ybiquitous marked this conversation as resolved.
Show resolved Hide resolved
stylelint: createEmptyStylelintPostcssResult(),
})
: createEmptyPostcssResult(inputFilePath);
}
}

/** @type {StylelintPostcssResult} */
const stylelintResult = {
ruleSeverities: {},
customMessages: {},
ruleMetadata: {},
disabledRanges: {},
fileCache,
};

const postcssResult =
Expand Down Expand Up @@ -104,6 +123,7 @@ function createEmptyStylelintPostcssResult() {
disabledRanges: {},
ignored: true,
stylelintError: false,
fileCache: undefined,
};
}

Expand Down
24 changes: 8 additions & 16 deletions lib/standalone.js
Expand Up @@ -14,11 +14,9 @@ const filterFilePaths = require('./utils/filterFilePaths');
const formatters = require('./formatters');
const getFileIgnorer = require('./utils/getFileIgnorer');
const getFormatterOptionsText = require('./utils/getFormatterOptionsText');
const hash = require('./utils/hash');
const NoFilesFoundError = require('./utils/noFilesFoundError');
const AllFilesIgnoredError = require('./utils/allFilesIgnoredError');
const { assert } = require('./utils/validateTypes');
const pkg = require('../package.json');
const prepareReturnValue = require('./prepareReturnValue');

const ALWAYS_IGNORED_GLOBS = ['**/node_modules/**'];
Expand Down Expand Up @@ -62,7 +60,7 @@ async function standalone({
syntax,
}) {
/** @type {FileCache} */
let fileCache;
let fileCache = new FileCache(cacheLocation, cwd);
const startTime = Date.now();

const isValidCode = typeof code === 'string';
Expand Down Expand Up @@ -175,15 +173,7 @@ async function standalone({
fileList = fileList.concat(ALWAYS_IGNORED_GLOBS.map((glob) => `!${glob}`));
}

if (useCache) {
const stylelintVersion = pkg.version;
const hashOfConfig = hash(`${stylelintVersion}_${JSON.stringify(config || {})}`);

fileCache = new FileCache(cacheLocation, cwd, hashOfConfig);
} else {
// No need to calculate hash here, we just want to delete cache file.
fileCache = new FileCache(cacheLocation, cwd);
// Remove cache file if cache option is disabled
if (!useCache) {
fileCache.destroy();
}

Expand Down Expand Up @@ -217,18 +207,20 @@ async function standalone({
return absoluteFilepath;
});

if (useCache) {
absoluteFilePaths = absoluteFilePaths.filter(fileCache.hasFileChanged.bind(fileCache));
}

const getStylelintResults = absoluteFilePaths.map(async (absoluteFilepath) => {
debug(`Processing ${absoluteFilepath}`);

try {
const postcssResult = await stylelint._lintSource({
filePath: absoluteFilepath,
cache: useCache,
fileCache,
});

if (postcssResult.stylelint.fileCache) {
fileCache = postcssResult.stylelint.fileCache;
}

if (postcssResult.stylelint.stylelintError && useCache) {
debug(`${absoluteFilepath} contains linting errors and will not be cached.`);
fileCache.removeEntry(absoluteFilepath);
Expand Down
7 changes: 7 additions & 0 deletions lib/utils/FileCache.js
Expand Up @@ -28,6 +28,13 @@ class FileCache {
this._hashOfConfig = hashOfConfig;
}
ybiquitous marked this conversation as resolved.
Show resolved Hide resolved

/**
* @param {string} hashOfConfig
*/
set hashOfConfig(hashOfConfig) {
this._hashOfConfig = hashOfConfig;
}
ybiquitous marked this conversation as resolved.
Show resolved Hide resolved

/**
* @param {string} absoluteFilepath
* @return {boolean}
Expand Down
14 changes: 14 additions & 0 deletions types/stylelint/index.d.ts
Expand Up @@ -2,6 +2,7 @@ declare module 'stylelint' {
import type * as PostCSS from 'postcss';
import type { GlobbyOptions } from 'globby';
import type { cosmiconfig } from 'cosmiconfig';
import type * as fileEntryCache from 'file-entry-cache';

namespace stylelint {
export type Severity = 'warning' | 'error';
Expand Down Expand Up @@ -87,6 +88,16 @@ declare module 'stylelint' {

export type DisabledWarning = { line: number; rule: string };

type FileCache = {
_fileCache: fileEntryCache.FileEntryCache;
_hashOfConfig: string;
ybiquitous marked this conversation as resolved.
Show resolved Hide resolved
hashOfConfig: string;
hasFileChanged: (absoluteFilepath: string) => boolean;
reconcile: () => void;
destroy: () => void;
removeEntry: (absoluteFilepath: string) => void;
};

export type StylelintPostcssResult = {
ruleSeverities: { [ruleName: string]: Severity };
customMessages: { [ruleName: string]: RuleMessage };
Expand All @@ -99,6 +110,7 @@ declare module 'stylelint' {
disableWritingFix?: boolean;
config?: Config;
ruleDisableFix?: boolean;
fileCache?: FileCache;
};

type EmptyResult = {
Expand Down Expand Up @@ -205,6 +217,8 @@ declare module 'stylelint' {

export type GetLintSourceOptions = GetPostcssOptions & {
existingPostcssResult?: PostCSS.Result;
cache?: boolean;
fileCache?: FileCache;
ybiquitous marked this conversation as resolved.
Show resolved Hide resolved
};

export type LinterOptions = {
Expand Down