From 037c4d51c0fdba61fc6149d0fc37ea612177c508 Mon Sep 17 00:00:00 2001 From: Jairo Panduro Date: Wed, 1 Mar 2023 18:23:53 +0100 Subject: [PATCH] feat: strf-10477, strf-10457 Base-level rules and conditional multi file import --- .github/workflows/pull_request_review.yml | 2 +- bin/stencil-scss-autofix.js | 2 +- lib/NodeSassAutoFixer.js | 280 ---------------------- lib/bundle-validator.js | 2 +- lib/nodeSass/AutoFixer.js | 122 ++++++++++ lib/nodeSass/BaseFixer.js | 56 +++++ lib/nodeSass/BaseRulesFixer.js | 28 +++ lib/nodeSass/ConditionalImportFixer.js | 134 +++++++++++ 8 files changed, 343 insertions(+), 283 deletions(-) delete mode 100644 lib/NodeSassAutoFixer.js create mode 100644 lib/nodeSass/AutoFixer.js create mode 100644 lib/nodeSass/BaseFixer.js create mode 100644 lib/nodeSass/BaseRulesFixer.js create mode 100644 lib/nodeSass/ConditionalImportFixer.js diff --git a/.github/workflows/pull_request_review.yml b/.github/workflows/pull_request_review.yml index 7ecc6906..bf40644a 100644 --- a/.github/workflows/pull_request_review.yml +++ b/.github/workflows/pull_request_review.yml @@ -5,7 +5,7 @@ name: Tests on: pull_request: - branches: [master, main] + branches: [master, main, release-**] jobs: build: diff --git a/bin/stencil-scss-autofix.js b/bin/stencil-scss-autofix.js index 37bbceb3..23fe5f6c 100644 --- a/bin/stencil-scss-autofix.js +++ b/bin/stencil-scss-autofix.js @@ -4,7 +4,7 @@ require('colors'); const program = require('../lib/commander'); const ThemeConfig = require('../lib/theme-config'); -const NodeSassAutoFixer = require('../lib/NodeSassAutoFixer'); +const NodeSassAutoFixer = require('../lib/nodeSass/AutoFixer'); const { THEME_PATH, PACKAGE_INFO } = require('../constants'); const { printCliResultErrorAndExit } = require('../lib/cliCommon'); diff --git a/lib/NodeSassAutoFixer.js b/lib/NodeSassAutoFixer.js deleted file mode 100644 index cdba2e85..00000000 --- a/lib/NodeSassAutoFixer.js +++ /dev/null @@ -1,280 +0,0 @@ -/* eslint-disable no-param-reassign, operator-assignment */ -require('colors'); - -const fs = require('fs'); -const path = require('path'); -const postcss = require('postcss'); -const postcssScss = require('postcss-scss'); - -const ScssValidator = require('./ScssValidator'); -const cssCompiler = require('./css/compile'); - -const CONDITIONAL_IMPORT = 'conditional-import'; - -class NodeSassAutoFixer { - /** - * - * @param themePath - * @param themeConfig - * @param cliOptions - */ - constructor(themePath, themeConfig, cliOptions) { - this.themePath = themePath; - this.themeConfig = themeConfig; - this.cliOptions = cliOptions; - - this.validator = new ScssValidator(themePath, themeConfig); - } - - async run() { - const assetsPath = path.join(this.themePath, 'assets'); - const rawConfig = await this.themeConfig.getConfig(); - const cssFiles = await this.validator.getCssFiles(); - - let issuesDetected = false; - /* eslint-disable-next-line no-useless-catch */ - try { - for await (const file of cssFiles) { - try { - /* eslint-disable-next-line no-await-in-loop */ - await cssCompiler.compile( - rawConfig, - assetsPath, - file, - cssCompiler.SASS_ENGINE_NAME, - ); - } catch (e) { - issuesDetected = true; - await this.tryToFix(e, file); - } - } - if (!issuesDetected) { - console.log('No issues deteted'); - } - } catch (e) { - throw e; - } - } - - async tryToFix(err, file) { - const problem = this.detectProblem(err); - if (problem) { - const dirname = path.join(this.themePath, 'assets/scss'); - const filePath = this.resolveScssFileName(dirname, err.file); - if (problem === CONDITIONAL_IMPORT) { - await this.fixConditionalImport(filePath); - } - } else { - const filePath = path.join(this.themePath, 'assets/scss', file + '.scss'); - console.log("Couldn't determine and autofix the problem. Please fix it manually.".red); - console.log('Found trying compile file:'.red, filePath); - throw new Error(err); - } - } - - detectProblem(err) { - if (err.formatted) { - if ( - err.formatted.includes( - 'Error: Import directives may not be used within control directives or mixins', - ) - ) { - return CONDITIONAL_IMPORT; - } - } - - return null; - } - - async fixConditionalImport(filePath) { - const scss = fs.readFileSync(filePath, 'utf8'); - const condImportFile = await this.processCss(scss, this.transformConditionalImport()); - - this.overrideFile(filePath, condImportFile.css); - // console.log(condImportFile.messages); - // throw new Error('1'); - for await (const message of condImportFile.messages) { - if (message.type === 'import') { - const importFile = this.findImportedFile(message.filename, filePath); - const importFileScss = fs.readFileSync(importFile); - const mixinFile = await this.processCss( - importFileScss, - this.transformRootToMixin(message), - ); - this.overrideFile(importFile, mixinFile.css); - } - } - } - - transformConditionalImport() { - return { - postcssPlugin: 'Transform Conditional Import', - AtRule: (rule, { AtRule, result }) => { - if (rule.name === 'if' || rule.name === 'else') { - rule.walkAtRules('import', (decl) => { - const newRule = new AtRule({ - name: 'import', - params: decl.params, - source: decl.source, - }); - const root = decl.root(); - root.prepend(newRule); - decl.name = 'include'; - //remove quotes from import - const oldName = decl.params.replace(/['"]+/g, ''); - // replace slash with dash - decl.params = oldName.replace(/\//g, '-') - result.messages.push({ - type: 'import', - filename: oldName, - }); - }); - } - }, - }; - } - - transformRootToMixin(data) { - const self = this; - return { - postcssPlugin: 'Transform Root to Mixin', - Once(root, { AtRule, result }) { - const mixins = []; - root.walkAtRules('mixin', (decl) => { - mixins.push(decl.clone()); - decl.remove(); - }); - // already wrapped in mixin - if ( - root.nodes.length === 1 && - root.nodes[0].type === 'atrule' && - root.nodes[0].name === 'mixin' - ) { - return; - } - const nodes = root.nodes.map((node) => { - const cloned = node.clone(); - if (!cloned) { - console.log('here3'); - console.log(root); - } - cloned.raws.before = "\n"; - return cloned; - }); - self.formatNodes(nodes); - const newRoot = new AtRule({ - name: 'mixin', - params: data.filename.replace(/\//g, '_'), - source: root.source, - nodes, - }); - if (mixins.length > 0) { - newRoot.raws.before = "\n"; - } - result.root.nodes = [...mixins, newRoot]; - } - }; - } - - formatNodes(nodes) { - if (nodes.length > 0) { - const spacer = this.getSpacer(nodes[0]); - this.addTabsToNodes(nodes, spacer); - } - } - - addTabsToNodes(nodes, spacer) { - nodes.forEach((node) => { - if (!node) { - console.log(nodes); - console.log('here'); - } - if (node.nodes) { - node.raws.before = node.raws.before + spacer; - node.raws.after = node.raws.after + spacer; - this.addTabsToNodes(node.nodes, spacer); - } else { - if (node.raws.before) { - node.raws.before = node.raws.before + spacer; - } - if (node.prop === 'src') { - node.value = node.value.replace(/\n/g, '\n' + spacer); - } - } - }); - } - - getSpacer(node) { - if (!node) { - console.log(node); - console.log('here2'); - } - const hasTabSpace = this.hasTabSpace(node.raws.before); - if (hasTabSpace) { - return '\t'; - } - - return ' '; - } - - hasTabSpace(string) { - return /\t/g.test(string); - } - - async processCss(data, plugin) { - const processor = postcss([plugin]); - return processor.process(data, { from: undefined, parser: postcssScss }); - } - - findImportedFile(file, originalFilePath) { - const originalDirname = path.dirname(originalFilePath); - return this.resolveScssFileName(originalDirname, file); - } - - resolveScssFileName(dirname, fileName) { - if (!fileName.includes('.scss')) { - fileName += '.scss'; - } - const filePath = path.join(dirname, fileName); - if (!fs.existsSync(filePath)) { - // try with underscore - const withUnderscoreFileName = this.getFileNameWithUnderscore(fileName); - const filePathWithUnderscore = path.join(dirname, withUnderscoreFileName); - if (!fs.existsSync(filePathWithUnderscore)) { - throw new Error( - `Import ${fileName} wasn't resolved in ${filePath} or ${filePathWithUnderscore}`, - ); - } - return filePathWithUnderscore; - } - - return filePath; - } - - getFileNameWithUnderscore(fileName) { - const fileNameParts = fileName.split('/'); - const withUnderscoreFileName = fileNameParts - .map((part, i) => { - if (i === fileNameParts.length - 1) { - return '_' + part; - } - return part; - }) - .join('/'); - return withUnderscoreFileName; - } - - overrideFile(filePath, data) { - const phrase = this.cliOptions.dry ? 'Would override' : 'Overriding'; - console.log(phrase.green + ' file:'.green, filePath); - if (this.cliOptions.dry) { - console.log('---Content---'.yellow); - console.log(data); - console.log('---END of Content---'.yellow); - } else { - fs.writeFileSync(filePath, data); - } - } -} - -module.exports = NodeSassAutoFixer; diff --git a/lib/bundle-validator.js b/lib/bundle-validator.js index fd7485d2..92327741 100644 --- a/lib/bundle-validator.js +++ b/lib/bundle-validator.js @@ -47,7 +47,7 @@ class BundleValidator { this.validationTasks = [ this._validateThemeConfiguration.bind(this), this._validateThemeSchema.bind(this), - // this._validateSchemaTranslations.bind(this), + this._validateSchemaTranslations.bind(this), this._validateTemplatesFrontmatter.bind(this), this._validateCssFiles.bind(this), ]; diff --git a/lib/nodeSass/AutoFixer.js b/lib/nodeSass/AutoFixer.js new file mode 100644 index 00000000..6912822a --- /dev/null +++ b/lib/nodeSass/AutoFixer.js @@ -0,0 +1,122 @@ +/* eslint-disable no-param-reassign, operator-assignment */ +require('colors'); + +const fs = require('fs'); +const path = require('path'); + +const ScssValidator = require('../ScssValidator'); +const cssCompiler = require('../css/compile'); + +const BaseRulesFixer = require('./BaseRulesFixer'); +const ConditionaImportFixer = require('./ConditionalImportFixer'); + +const CONDITIONAL_IMPORT = 'conditional-import'; +const BASE_LEVEL_RULES = 'base-level-rules'; + +class AutoFixer { + /** + * + * @param themePath + * @param themeConfig + * @param cliOptions + */ + constructor(themePath, themeConfig, cliOptions) { + this.themePath = themePath; + this.themeConfig = themeConfig; + this.cliOptions = cliOptions; + + this.validator = new ScssValidator(themePath, themeConfig); + } + + async run() { + const assetsPath = path.join(this.themePath, 'assets'); + const rawConfig = await this.themeConfig.getConfig(); + const cssFiles = await this.validator.getCssFiles(); + + let issuesDetected = false; + /* eslint-disable-next-line no-useless-catch */ + try { + for await (const file of cssFiles) { + try { + /* eslint-disable-next-line no-await-in-loop */ + await cssCompiler.compile( + rawConfig, + assetsPath, + file, + cssCompiler.SASS_ENGINE_NAME, + ); + } catch (e) { + issuesDetected = true; + await this.tryToFix(e, file); + } + } + if (!issuesDetected) { + console.log('No issues deteted'); + } + } catch (e) { + throw e; + } + } + + async tryToFix(err, file) { + const problem = this.detectProblem(err); + if (problem) { + const dirname = path.join(this.themePath, 'assets/scss'); + let files = []; + if (problem === CONDITIONAL_IMPORT) { + const fixer = new ConditionaImportFixer(dirname, err.file); + files = await fixer.run(); + } else if (problem === BASE_LEVEL_RULES) { + const baseRulesFixer = new BaseRulesFixer(dirname, err.file); + files = await baseRulesFixer.run(); + } + this.parseChangedFiles(files); + } else { + const filePath = path.join(this.themePath, 'assets/scss', file + '.scss'); + console.log("Couldn't determine and autofix the problem. Please fix it manually.".red); + console.log('Found trying compile file:'.red, filePath); + throw new Error(err); + } + } + + detectProblem(err) { + if (err.formatted) { + if ( + err.formatted.includes( + 'Error: Import directives may not be used within control directives or mixins', + ) + ) { + return CONDITIONAL_IMPORT; + } + if ( + err.formatted.includes( + "Base-level rules cannot contain the parent-selector-referencing character '&'", + ) + ) { + return BASE_LEVEL_RULES; + } + } + + return null; + } + + parseChangedFiles(files) { + for (const file of files) { + this.overrideFile(file.filePath, file.data); + } + } + + overrideFile(filePath, data) { + const phrase = this.cliOptions.dry ? 'Would override' : 'Overriding'; + console.log(phrase.green + ' file:'.green, filePath); + if (this.cliOptions.dry) { + console.log('---Content---'.yellow); + console.log(data); + console.log('---END of Content---'.yellow); + } else { + fs.writeFileSync(filePath, data); + } + } +} + +module.exports = AutoFixer; diff --git a/lib/nodeSass/BaseFixer.js b/lib/nodeSass/BaseFixer.js new file mode 100644 index 00000000..b00c7547 --- /dev/null +++ b/lib/nodeSass/BaseFixer.js @@ -0,0 +1,56 @@ +const fs = require('fs'); +const path = require('path'); +const postcss = require('postcss'); +const postcssScss = require('postcss-scss'); + +class BaseFixer { + constructor(dirname, brokenFile) { + this.filePath = this.resolveScssFileName(dirname, brokenFile); + } + + async processCss(data, plugin) { + const processor = postcss([plugin]); + return processor.process(data, { from: undefined, parser: postcssScss }); + } + + findImportedFile(file, originalFilePath) { + const originalDirname = path.dirname(originalFilePath); + return this.resolveScssFileName(originalDirname, file); + } + + resolveScssFileName(dirname, fileName) { + if (!fileName.includes('.scss')) { + /* eslint-disable-next-line no-param-reassign */ + fileName += '.scss'; + } + const filePath = path.join(dirname, fileName); + if (!fs.existsSync(filePath)) { + // try with underscore + const fileNameWithUnderscore = this.getFileNameWithUnderscore(fileName); + const filePathWithUnderscore = path.join(dirname, fileNameWithUnderscore); + if (!fs.existsSync(filePathWithUnderscore)) { + throw new Error( + `Import ${fileName} wasn't resolved in ${filePath} or ${filePathWithUnderscore}`, + ); + } + return filePathWithUnderscore; + } + + return filePath; + } + + getFileNameWithUnderscore(fileName) { + const fileNameParts = fileName.split('/'); + const fileNameWithUnderscore = fileNameParts + .map((part, i) => { + if (i === fileNameParts.length - 1) { + return '_' + part; + } + return part; + }) + .join('/'); + return fileNameWithUnderscore; + } +} + +module.exports = BaseFixer; diff --git a/lib/nodeSass/BaseRulesFixer.js b/lib/nodeSass/BaseRulesFixer.js new file mode 100644 index 00000000..e983a888 --- /dev/null +++ b/lib/nodeSass/BaseRulesFixer.js @@ -0,0 +1,28 @@ +// Since previously a broken css was produced which had no effect on the page (browsers are smart enough to ignore unprocessable css) +// we can just comment out that part of code + +const fs = require('fs'); +const BaseFixer = require('./BaseFixer'); + +class BaseRulesFixer extends BaseFixer { + async run() { + const scss = fs.readFileSync(this.filePath, 'utf8'); + const processedFile = await this.processCss(scss, this.transform()); + + return [{ filePath: this.filePath, data: processedFile.css }]; + } + + transform() { + return { + postcssPlugin: 'Transform Base Rules Issues into Comments', + Rule(rule, { Comment }) { + if (rule.parent.type === 'root' && rule.selector.startsWith('&--')) { + const comment = new Comment({ text: rule.toString() }); + rule.replaceWith(comment); + } + }, + }; + } +} + +module.exports = BaseRulesFixer; diff --git a/lib/nodeSass/ConditionalImportFixer.js b/lib/nodeSass/ConditionalImportFixer.js new file mode 100644 index 00000000..2e64610a --- /dev/null +++ b/lib/nodeSass/ConditionalImportFixer.js @@ -0,0 +1,134 @@ +/* eslint-disable no-param-reassign, operator-assignment */ +const fs = require('fs'); + +const BaseFixer = require('./BaseFixer'); + +class ConditionalImportFixer extends BaseFixer { + async run() { + const files = []; + const scss = fs.readFileSync(this.filePath, 'utf8'); + const condImportFile = await this.processCss(scss, this.transformConditionalImport()); + + files.push({ filePath: this.filePath, data: condImportFile.css }); + for await (const message of condImportFile.messages) { + if (message.type === 'import') { + const importFile = this.findImportedFile(message.filename, this.filePath); + const importFileScss = fs.readFileSync(importFile); + const mixinFile = await this.processCss( + importFileScss, + this.transformRootToMixin(message), + ); + files.push({ + filePath: importFile, + data: mixinFile.css, + }); + } + } + return files; + } + + transformConditionalImport() { + return { + postcssPlugin: 'Transform Conditional Import', + AtRule: (rule, { AtRule, result }) => { + if (rule.name === 'if' || rule.name === 'else') { + rule.walkAtRules('import', (decl) => { + const newRule = new AtRule({ + name: 'import', + params: decl.params, + source: decl.source, + }); + const root = decl.root(); + root.prepend(newRule); + decl.name = 'include'; + // remove quotes from import + const oldName = decl.params.replace(/['"]+/g, ''); + // replace slash with dash + decl.params = oldName.replace(/\//g, '-'); + result.messages.push({ + type: 'import', + filename: oldName, + }); + }); + } + }, + }; + } + + transformRootToMixin(data) { + const self = this; + return { + postcssPlugin: 'Transform Root to Mixin', + Once(root, { AtRule, result }) { + const mixins = []; + root.walkAtRules('mixin', (decl) => { + mixins.push(decl.clone()); + decl.remove(); + }); + // already wrapped in mixin + if ( + root.nodes.length === 1 && + root.nodes[0].type === 'atrule' && + root.nodes[0].name === 'mixin' + ) { + return; + } + const nodes = root.nodes.map((node) => { + const cloned = node.clone(); + cloned.raws.before = '\n'; + return cloned; + }); + self.formatNodes(nodes); + const newRoot = new AtRule({ + name: 'mixin', + params: data.filename.replace(/\//g, '_'), + source: root.source, + nodes, + }); + if (mixins.length > 0) { + newRoot.raws.before = '\n'; + } + result.root.nodes = [...mixins, newRoot]; + }, + }; + } + + formatNodes(nodes) { + if (nodes.length > 0) { + const spacer = this.getSpacer(nodes[0]); + this.addTabsToNodes(nodes, spacer); + } + } + + addTabsToNodes(nodes, spacer) { + nodes.forEach((node) => { + if (node.nodes) { + node.raws.before = node.raws.before + spacer; + node.raws.after = node.raws.after + spacer; + this.addTabsToNodes(node.nodes, spacer); + } else { + if (node.raws.before) { + node.raws.before = node.raws.before + spacer; + } + if (node.prop === 'src') { + node.value = node.value.replace(/\n/g, '\n' + spacer); + } + } + }); + } + + getSpacer(node) { + const hasTabSpace = this.hasTabSpace(node.raws.before); + if (hasTabSpace) { + return '\t'; + } + + return ' '; + } + + hasTabSpace(string) { + return /\t/g.test(string); + } +} + +module.exports = ConditionalImportFixer;