From bd12ca2001db05a814e34b294a9a699731e49bd2 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Fri, 19 Mar 2021 07:47:02 +0800 Subject: [PATCH] MDL-68496 Grunt: Stylelint should only lint relevant component files 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. --- .grunt/components.js | 11 +++ .grunt/tasks/sass.js | 9 -- .grunt/tasks/stylelint.js | 183 ++++++++++++++++++++++---------------- Gruntfile.js | 54 ++++++++++- 4 files changed, 169 insertions(+), 88 deletions(-) diff --git a/.grunt/components.js b/.grunt/components.js index 8b8291deaa4..848632db6dd 100644 --- a/.grunt/components.js +++ b/.grunt/components.js @@ -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. * @@ -225,6 +235,7 @@ const getOwningComponentDirectory = checkPath => { module.exports = { getAmdSrcGlobList, getComponentFromPath, + getComponentPaths, getOwningComponentDirectory, getYuiSrcGlobList, getThirdPartyLibsList, diff --git a/.grunt/tasks/sass.js b/.grunt/tasks/sass.js index 7124cbdf287..cdb8bb1fe15 100644 --- a/.grunt/tasks/sass.js +++ b/.grunt/tasks/sass.js @@ -37,13 +37,4 @@ module.exports = grunt => { } }, }); - - grunt.config.merge({ - watch: { - boost: { - files: [grunt.moodleEnv.inComponent ? 'scss/**/*.scss' : 'theme/boost/scss/**/*.scss'], - tasks: ['scss'] - }, - }, - }); }; diff --git a/.grunt/tasks/stylelint.js b/.grunt/tasks/stylelint.js index 22f04cdd910..4da9067e644 100644 --- a/.grunt/tasks/stylelint.js +++ b/.grunt/tasks/stylelint.js @@ -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. * @@ -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; + // 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 = []; - 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'); + 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. - // CSS exists in: - // [component]/styles.css - // [theme_pluginname]/css + requestedFiles.forEach(changedFilePath => { + let matchesGlob; - 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; - } - } - } 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 SCSS file. + matchesGlob = grunt.moodleEnv.scssSrc.some(watchedPathGlob => { + return glob.sync(watchedPathGlob).indexOf(changedFilePath) !== -1; + }); + if (matchesGlob) { + scssFiles.push(changedFilePath); hasScss = true; } - } - const cssSrc = []; - glob.sync(`${fullRunDir}/**/*.css`).forEach(path => cssSrc.push(path)); + // 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. - if (cssSrc.length) { - grunt.config.merge(getCssConfigForFiles(cssSrc)); - hasCss = true; - } + grunt.moodleEnv.scssSrc.forEach(path => { + if (path.startsWith(grunt.moodleEnv.runDir)) { + scssFiles.push(path); + hasScss = 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: { @@ -143,8 +172,10 @@ module.exports = grunt => { ], tasks: ['rawcss'] }, + scss: { + files: getCoreThemeMatches(), + tasks: ['scss'] + }, }, }); - - registerStyleLintTasks(); }; diff --git a/Gruntfile.js b/Gruntfile.js index cdc6534ad0f..89836af0e4f 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -68,6 +68,54 @@ 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). * @@ -75,9 +123,6 @@ const setupMoodleEnvironment = 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 @@ -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')) { @@ -143,6 +189,7 @@ const setupMoodleEnvironment = grunt => { amdSrc, componentDirectory, cwd, + cssSrc, files, fullRunDir, gruntFilePath, @@ -151,6 +198,7 @@ const setupMoodleEnvironment = grunt => { inTheme, relativeCwd, runDir, + scssSrc, yuiSrc, }; };