Skip to content

Commit

Permalink
MDL-68496 Grunt: Stylelint should only lint relevant component files
Browse files Browse the repository at this point in the history
Prior to this change the Grunt stylelint command was too greedy when
determining which files hsould be linted.

This change modifies the watch command to only watch relevant files and
subdirectories of each component directories. This means that unrelated
CSS and SCSS files are no longer watched for changes, and has the added
benefit of significantly increaseing the startup speed of grunt.

Without this patch applied the watch tasks were checking for matches in
the node_modules, and vendor directories.
  • Loading branch information
andrewnicols committed Mar 22, 2021
1 parent 61fca0e commit 32638b3
Show file tree
Hide file tree
Showing 4 changed files with 171 additions and 90 deletions.
11 changes: 11 additions & 0 deletions .grunt/components.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,16 @@ const fetchComponentData = () => {
return componentData;
};

/**
* Get the list of component paths.
*
* @param {string} relativeTo
* @returns {array}
*/
const getComponentPaths = (relativeTo = '') => fetchComponentData().pathList.map(componentPath => {
return componentPath.replace(relativeTo, '');
});

/**
* Get the list of paths to build AMD sources.
*
Expand Down Expand Up @@ -225,6 +235,7 @@ const getOwningComponentDirectory = checkPath => {
module.exports = {
getAmdSrcGlobList,
getComponentFromPath,
getComponentPaths,
getOwningComponentDirectory,
getYuiSrcGlobList,
getThirdPartyLibsList,
Expand Down
9 changes: 0 additions & 9 deletions .grunt/tasks/sass.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,4 @@ module.exports = grunt => {
}
},
});

grunt.config.merge({
watch: {
boost: {
files: [grunt.moodleEnv.inComponent ? 'scss/**/*.scss' : 'theme/boost/scss/**/*.scss'],
tasks: ['scss']
},
},
});
};
187 changes: 109 additions & 78 deletions .grunt/tasks/stylelint.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,37 @@
*/

module.exports = grunt => {

const getCssConfigForFiles = files => {
return {
stylelint: {
css: {
// Use a fully-qualified path.
src: files,
options: {
configOverrides: {
rules: {
// These rules have to be disabled in .stylelintrc for scss compat.
"at-rule-no-unknown": true,
}
}
}
},
},
};
};

const getScssConfigForFiles = files => {
return {
stylelint: {
scss: {
options: {syntax: 'scss'},
src: files,
},
},
};
};

/**
* Register any stylelint tasks.
*
Expand All @@ -29,108 +60,106 @@ module.exports = grunt => {
* @param {String} fullRunDir
*/
const registerStyleLintTasks = () => {
const files = grunt.moodleEnv.files;
const fullRunDir = grunt.moodleEnv.fullRunDir;
const inComponent = grunt.moodleEnv.inComponent;
const inTheme = grunt.moodleEnv.inTheme;

const getCssConfigForFiles = files => {
return {
stylelint: {
css: {
// Use a fully-qualified path.
src: files,
options: {
configOverrides: {
rules: {
// These rules have to be disabled in .stylelintrc for scss compat.
"at-rule-no-unknown": true,
}
}
}
},
},
};
};

const getScssConfigForFiles = files => {
return {
stylelint: {
scss: {
options: {syntax: 'scss'},
src: files,
},
},
};
};
const glob = require('glob');

// The stylelinters do not handle the case where a configuration was provided but no files were included.
// Keep track of whether any files were found.
let hasCss = false;
let hasScss = false;

if (files) {
// Specific files were passed. Just set them up.
grunt.config.merge(getCssConfigForFiles(files));
hasCss = true;

grunt.config.merge(getScssConfigForFiles(files));
hasScss = true;
} else {
// The stylelint system does not handle the case where there was no file to lint.
// Check whether there are any files to lint in the current directory.
const glob = require('glob');

// CSS exists in:
// [component]/styles.css
// [theme_pluginname]/css

if (inComponent) {
hasScss = false;
if (inTheme) {
const scssSrc = [];
glob.sync(`${fullRunDir}/**/*.scss`).forEach(path => scssSrc.push(path));

if (scssSrc.length) {
grunt.config.merge(getScssConfigForFiles(scssSrc));
hasScss = true;
}
// The stylelint processors do not take a path argument. They always check all provided values.
// As a result we must check through each glob and determine if any files match the current directory.
const scssFiles = [];
const cssFiles = [];

const requestedFiles = grunt.moodleEnv.files;
if (requestedFiles) {
// Grunt was called with a files argument.
// Check whether each of the requested files matches either the CSS or SCSS source file list.

requestedFiles.forEach(changedFilePath => {
let matchesGlob;

// Check whether this watched path matches any watched SCSS file.
matchesGlob = grunt.moodleEnv.scssSrc.some(watchedPathGlob => {
return glob.sync(watchedPathGlob).indexOf(changedFilePath) !== -1;
});
if (matchesGlob) {
scssFiles.push(changedFilePath);
hasScss = true;
}
} else {
const scssSrc = [];
glob.sync(`${fullRunDir}/**/*.scss`).forEach(path => scssSrc.push(path));

if (scssSrc.length) {
grunt.config.merge(getScssConfigForFiles(scssSrc));
// Check whether this watched path matches any watched CSS file.
matchesGlob = grunt.moodleEnv.cssSrc.some(watchedPathGlob => {
return glob.sync(watchedPathGlob).indexOf(changedFilePath) !== -1;
});
if (matchesGlob) {
cssFiles.push(changedFilePath);
hasCss = true;
}
});
} else {
// Grunt was called without a list of files.
// The start directory (runDir) may be a child dir of the project.
// Check each scssSrc file to see if it's in the start directory.
// This means that we can lint just mod/*/styles.css if started in the mod directory.

grunt.moodleEnv.scssSrc.forEach(path => {
if (path.startsWith(grunt.moodleEnv.runDir)) {
scssFiles.push(path);
hasScss = true;
}
}

const cssSrc = [];
glob.sync(`${fullRunDir}/**/*.css`).forEach(path => cssSrc.push(path));
});

if (cssSrc.length) {
grunt.config.merge(getCssConfigForFiles(cssSrc));
hasCss = true;
}
grunt.moodleEnv.cssSrc.forEach(path => {
if (path.startsWith(grunt.moodleEnv.runDir)) {
cssFiles.push(path);
hasCss = true;
}
});
}

// Register the tasks.
const scssTasks = ['sass'];
if (hasScss) {
grunt.config.merge(getScssConfigForFiles(scssFiles));
scssTasks.unshift('stylelint:scss');
}
grunt.registerTask('scss', scssTasks);

const cssTasks = [];
if (hasCss) {
grunt.config.merge(getCssConfigForFiles(cssFiles));
cssTasks.push('stylelint:css');
}
grunt.registerTask('rawcss', cssTasks);

grunt.registerTask('css', ['scss', 'rawcss']);
// The tasks must be registered, even if empty to ensure a consistent command list.
// They jsut won't run anything.
grunt.registerTask('scss', scssTasks);
grunt.registerTask('rawcss', cssTasks);
};

// Register CSS tasks.
grunt.loadNpmTasks('grunt-stylelint');

// Register the style lint tasks.
registerStyleLintTasks();
grunt.registerTask('css', ['scss', 'rawcss']);

const getCoreThemeMatches = () => {
const scssMatch = 'scss/**/*.scss';

if (grunt.moodleEnv.inTheme) {
return [scssMatch];
}

if (grunt.moodleEnv.runDir.startsWith('theme')) {
return [`*/${scssMatch}`];
}

return [`theme/*/${scssMatch}`];
};

// Add the watch configuration for rawcss, and scss.
grunt.config.merge({
watch: {
rawcss: {
Expand All @@ -143,8 +172,10 @@ module.exports = grunt => {
],
tasks: ['rawcss']
},
scss: {
files: getCoreThemeMatches(),
tasks: ['scss']
},
},
});

registerStyleLintTasks();
};
54 changes: 51 additions & 3 deletions Gruntfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,16 +68,61 @@ const setupMoodleEnvironment = grunt => {
};
};

const getStyleConfiguration = () => {
const ComponentList = require(path.join(process.cwd(), '.grunt', 'components.js'));
// Build the cssSrc and scssSrc.
// Valid paths are:
// [component]/styles.css; and either
// [theme/[themename]]/scss/**/*.scss; or
// [theme/[themename]]/style/*.css.
//
// If a theme has scss, then it is assumed that the style directory contains generated content.
let cssSrc = [];
let scssSrc = [];

const checkComponentDirectory = componentDirectory => {
const isTheme = componentDirectory.startsWith('theme/');
if (isTheme) {
const scssDirectory = `${componentDirectory}/scss`;

if (fs.existsSync(scssDirectory)) {
// This theme has an SCSS directory.
// Include all scss files within it recursively, but do not check for css files.
scssSrc.push(`${scssDirectory}/*.scss`);
scssSrc.push(`${scssDirectory}/**/*.scss`);
} else {
// This theme has no SCSS directory.
// Only hte CSS files in the top-level directory are checked.
cssSrc.push(`${componentDirectory}/style/*.css`);
}
} else {
// This is not a theme.
// All other plugin types are restricted to a single styles.css in their top level.
cssSrc.push(`${componentDirectory}/styles.css`);
}
};

if (inComponent) {
checkComponentDirectory(componentDirectory);
} else {
ComponentList.getComponentPaths(`${gruntFilePath}/`).forEach(componentPath => {
checkComponentDirectory(componentPath);
});
}

return {
cssSrc,
scssSrc,
};
};

/**
* Calculate the cwd, taking into consideration the `root` option (for Windows).
*
* @param {Object} grunt
* @returns {String} The current directory as best we can determine
*/
const getCwd = grunt => {
const fs = require('fs');
const path = require('path');

let cwd = fs.realpathSync(process.env.PWD || process.cwd());

// Windows users can't run grunt in a subdirectory, so allow them to set
Expand Down Expand Up @@ -113,6 +158,7 @@ const setupMoodleEnvironment = grunt => {
const fullRunDir = fs.realpathSync(gruntFilePath + path.sep + runDir);
const {inAMD, amdSrc} = getAmdConfiguration();
const {yuiSrc} = getYuiConfiguration();
const {cssSrc, scssSrc} = getStyleConfiguration();

let files = null;
if (grunt.option('files')) {
Expand Down Expand Up @@ -143,6 +189,7 @@ const setupMoodleEnvironment = grunt => {
amdSrc,
componentDirectory,
cwd,
cssSrc,
files,
fullRunDir,
gruntFilePath,
Expand All @@ -151,6 +198,7 @@ const setupMoodleEnvironment = grunt => {
inTheme,
relativeCwd,
runDir,
scssSrc,
yuiSrc,
};
};
Expand Down

0 comments on commit 32638b3

Please sign in to comment.