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
60 changes: 54 additions & 6 deletions lib/__tests__/standalone-cache.test.js
Expand Up @@ -10,6 +10,10 @@ const removeFile = require('../testUtils/removeFile');
const safeChdir = require('../testUtils/safeChdir');
const standalone = require('../standalone');

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

const fixturesPath = path.join(__dirname, 'fixtures');
const invalidFile = path.join(fixturesPath, 'empty-block.css');
const syntaxErrorFile = path.join(fixturesPath, 'syntax_error.css');
Expand Down Expand Up @@ -68,8 +72,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 @@ -92,8 +96,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 @@ -105,8 +109,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 Expand Up @@ -194,3 +198,47 @@ describe('standalone cache uses cacheLocation', () => {
expect(cache.getKey(validFile)).toBeTruthy();
});
});

describe('standalone cache uses a config file', () => {
const cwd = path.join(__dirname, 'tmp', 'standalone-cache-use-config-file');

safeChdir(cwd);

const configFile = path.join(cwd, '.stylelintrc.json');
const lintedFile = path.join(cwd, 'a.css');

beforeEach(async () => {
await fs.writeFile(lintedFile, 'a {}');
});

afterEach(async () => {
await removeFile(configFile);
await removeFile(lintedFile);
});

it('cache is discarded when a config file is changed', async () => {
const config = { files: [lintedFile], config: undefined, cache: true };

// No warnings when a rule is disabled.
await fs.writeFile(
configFile,
JSON.stringify({
rules: { 'block-no-empty': null },
}),
);
const { results } = await standalone(config);

expect(results[0].warnings).toHaveLength(0);

// Some warnings when a rule becomes enabled by changing the config.
await fs.writeFile(
configFile,
JSON.stringify({
rules: { 'block-no-empty': true },
}),
);
const { results: resultsNew } = await standalone(config);

expect(resultsNew[0].warnings).toHaveLength(1);
});
});
2 changes: 2 additions & 0 deletions lib/createStylelint.js
Expand Up @@ -6,6 +6,7 @@ const getConfigForFile = require('./getConfigForFile');
const getPostcssResult = require('./getPostcssResult');
const isPathIgnored = require('./isPathIgnored');
const lintSource = require('./lintSource');
const FileCache = require('./utils/FileCache');
const { cosmiconfig } = require('cosmiconfig');

const IS_TEST = process.env.NODE_ENV === 'test';
Expand Down Expand Up @@ -35,6 +36,7 @@ function createStylelint(options = {}) {

stylelint._specifiedConfigCache = new Map();
stylelint._postcssResultCache = new Map();
stylelint._fileCache = new FileCache(stylelint._options.cacheLocation, stylelint._options.cwd);
stylelint._createStylelintResult = createStylelintResult.bind(null, stylelint);
stylelint._getPostcssResult = getPostcssResult.bind(null, stylelint);
stylelint._lintSource = lintSource.bind(null, stylelint);
Expand Down
12 changes: 12 additions & 0 deletions lib/lintSource.js
Expand Up @@ -66,6 +66,18 @@ module.exports = async function lintSource(stylelint, options = {}) {
const config = configForFile.config;
const existingPostcssResult = options.existingPostcssResult;

if (options.cache) {
stylelint._fileCache.calcHashOfConfig(config);

if (options.filePath && !stylelint._fileCache.hasFileChanged(options.filePath)) {
return existingPostcssResult
? Object.assign(existingPostcssResult, {
stylelint: createEmptyStylelintPostcssResult(),
})
: createEmptyPostcssResult(inputFilePath);
}
}

/** @type {StylelintPostcssResult} */
const stylelintResult = {
ruleSeverities: {},
Expand Down
29 changes: 7 additions & 22 deletions lib/standalone.js
Expand Up @@ -9,16 +9,13 @@ const path = require('path');

const createStylelint = require('./createStylelint');
const createStylelintResult = require('./createStylelintResult');
const FileCache = require('./utils/FileCache');
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 @@ -61,8 +58,6 @@ async function standalone({
reportNeedlessDisables,
syntax,
}) {
/** @type {FileCache} */
let fileCache;
const startTime = Date.now();

const isValidCode = typeof code === 'string';
Expand Down Expand Up @@ -95,6 +90,7 @@ async function standalone({
}

const stylelint = createStylelint({
cacheLocation,
config,
configFile,
configBasedir,
Expand Down Expand Up @@ -175,16 +171,8 @@ 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
fileCache.destroy();
if (!useCache) {
stylelint._fileCache.destroy();
}

const effectiveGlobbyOptions = {
Expand Down Expand Up @@ -217,21 +205,18 @@ 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,
});

if (postcssResult.stylelint.stylelintError && useCache) {
debug(`${absoluteFilepath} contains linting errors and will not be cached.`);
fileCache.removeEntry(absoluteFilepath);
stylelint._fileCache.removeEntry(absoluteFilepath);
}

/**
Expand All @@ -258,7 +243,7 @@ async function standalone({
return stylelint._createStylelintResult(postcssResult, absoluteFilepath);
} catch (error) {
// On any error, we should not cache the lint result
fileCache.removeEntry(absoluteFilepath);
stylelint._fileCache.removeEntry(absoluteFilepath);

return handleError(stylelint, error, absoluteFilepath);
}
Expand All @@ -275,7 +260,7 @@ async function standalone({
}

if (useCache) {
fileCache.reconcile();
stylelint._fileCache.reconcile();
}

const result = prepareReturnValue(stylelintResults, maxWarnings, formatterFunction, cwd);
Expand Down
32 changes: 20 additions & 12 deletions lib/utils/FileCache.js
Expand Up @@ -3,29 +3,37 @@
const debug = require('debug')('stylelint:file-cache');
const fileEntryCache = require('file-entry-cache');
const getCacheFile = require('./getCacheFile');
const hash = require('./hash');
const pkg = require('../../package.json');
const path = require('path');

const DEFAULT_CACHE_LOCATION = './.stylelintcache';
const DEFAULT_HASH = '';

/** @typedef {import('file-entry-cache').FileDescriptor["meta"] & { hashOfConfig?: string }} CacheMetadata */

/**
* @param {string} [cacheLocation]
* @param {string} [hashOfConfig]
* @constructor
*/
class FileCache {
constructor(
cacheLocation = DEFAULT_CACHE_LOCATION,
cwd = process.cwd(),
hashOfConfig = DEFAULT_HASH,
) {
/**
* @param {string | undefined} cacheLocation
* @param {string} cwd
*/
constructor(cacheLocation = DEFAULT_CACHE_LOCATION, cwd) {
const cacheFile = path.resolve(getCacheFile(cacheLocation, cwd));

debug(`Cache file is created at ${cacheFile}`);
this._fileCache = fileEntryCache.create(cacheFile);
this._hashOfConfig = hashOfConfig;
this._hashOfConfig = '';
}
ybiquitous marked this conversation as resolved.
Show resolved Hide resolved

/**
* @param {import('stylelint').Config} config
*/
calcHashOfConfig(config) {
if (this._hashOfConfig) return;

const stylelintVersion = pkg.version;
const configString = JSON.stringify(config || {});

this._hashOfConfig = hash(`${stylelintVersion}_${configString}`);
}
ybiquitous marked this conversation as resolved.
Show resolved Hide resolved

/**
Expand Down
11 changes: 11 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,14 @@ declare module 'stylelint' {

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

type FileCache = {
calcHashOfConfig: (config: Config) => void;
hasFileChanged: (absoluteFilepath: string) => boolean;
reconcile: () => void;
destroy: () => void;
removeEntry: (absoluteFilepath: string) => void;
};

export type StylelintPostcssResult = {
ruleSeverities: { [ruleName: string]: Severity };
customMessages: { [ruleName: string]: RuleMessage };
Expand Down Expand Up @@ -205,6 +214,7 @@ declare module 'stylelint' {

export type GetLintSourceOptions = GetPostcssOptions & {
existingPostcssResult?: PostCSS.Result;
cache?: boolean;
};

export type LinterOptions = {
Expand Down Expand Up @@ -517,6 +527,7 @@ declare module 'stylelint' {
_extendExplorer: ReturnType<typeof cosmiconfig>;
_specifiedConfigCache: Map<Config, Promise<CosmiconfigResult>>;
_postcssResultCache: Map<string, PostCSS.Result>;
_fileCache: FileCache;

_getPostcssResult: (options?: GetPostcssOptions) => Promise<PostCSS.Result>;
_lintSource: (options: GetLintSourceOptions) => Promise<PostcssResult>;
Expand Down