From 55da5c7ee2000671e29800d6101cf3c6c229d5e3 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Tue, 31 Aug 2021 16:38:24 +0200 Subject: [PATCH 1/8] [INTERNAL] Refactoring - Convert Promises to async/await - Use template literals --- lib/index.js | 241 ++++++++++++-------------- lib/plugin/css-variables-collector.js | 12 +- 2 files changed, 122 insertions(+), 131 deletions(-) diff --git a/lib/index.js b/lib/index.js index f833f8c5..1e639030 100644 --- a/lib/index.js +++ b/lib/index.js @@ -104,7 +104,7 @@ Builder.prototype.cacheTheme = function(result) { }; /** - * Creates a themebuild + * Runs a theme build * @param {object} options * @param {object} options.compiler compiler object as passed to less * @param {boolean} options.cssVariables whether or not to enable css variables output @@ -178,7 +178,7 @@ Builder.prototype.build = function(options) { }); } - function compile(config) { + async function compile(config) { const parserOptions = clone(options.parser); let rootpath; @@ -208,151 +208,138 @@ Builder.prototype.build = function(options) { const parser = createParser(parserOptions, fnFileHandler); - return new Promise(function(resolve, reject) { - parser.parse(config.content, function(err, tree) { - if (err) { - reject(err); - } else { - resolve(tree); - } + function parseContent(content) { + return new Promise(function(resolve, reject) { + parser.parse(content, function(err, tree) { + if (err) { + reject(err); + } else { + resolve(tree); + } + }); }); - }).then(async function(tree) { - const result = {}; - - result.tree = tree; + } - // plugins to collect imported files and variable values - const oImportCollector = new ImportCollectorPlugin({ - importMappings: mFileMappings[filename] - }); - const oVariableCollector = new VariableCollectorPlugin(options.compiler); - const oUrlCollector = new UrlCollector(); + const tree = await parseContent(config.content); + const result = {tree}; - // render to css - result.css = tree.toCSS(Object.assign({}, options.compiler, { - plugins: [oImportCollector, oVariableCollector, oUrlCollector] - })); + // plugins to collect imported files and variable values + const oImportCollector = new ImportCollectorPlugin({ + importMappings: mFileMappings[filename] + }); + const oVariableCollector = new VariableCollectorPlugin(options.compiler); + const oUrlCollector = new UrlCollector(); + + // render to css + result.css = tree.toCSS(Object.assign({}, options.compiler, { + plugins: [oImportCollector, oVariableCollector, oUrlCollector] + })); + + // retrieve imported files + result.imports = oImportCollector.getImports(); + + // retrieve reduced set of variables + result.variables = oVariableCollector.getVariables(Object.keys(mFileMappings[filename] || {})); + + // retrieve all variables + result.allVariables = oVariableCollector.getAllVariables(); + + // also compile rtl-version if requested + let oRTL; + if (options.rtl) { + const RTLPlugin = require("./plugin/rtl"); + oRTL = new RTLPlugin(); + + const urls = oUrlCollector.getUrls(); + + const existingImgRtlUrls = (await Promise.all( + urls.map(async ({currentDirectory, relativeUrl}) => { + const relativeImgRtlUrl = RTLPlugin.getRtlImgUrl(relativeUrl); + if (relativeImgRtlUrl) { + const resolvedImgRtlUrl = path.posix.join(currentDirectory, relativeImgRtlUrl); + if (await that.fileUtils.findFile(resolvedImgRtlUrl, options.rootPaths)) { + return resolvedImgRtlUrl; + } + } + }) + )).filter(Boolean); - // retrieve imported files - result.imports = oImportCollector.getImports(); + oRTL.setExistingImgRtlPaths(existingImgRtlUrls); + } - // retrieve reduced set of variables - result.variables = oVariableCollector.getVariables(Object.keys(mFileMappings[filename] || {})); + if (oRTL) { + result.cssRtl = tree.toCSS(Object.assign({}, options.compiler, { + plugins: [oRTL] + })); + } - // retrieve all variables - result.allVariables = oVariableCollector.getAllVariables(); + if (rootpath) { + result.imports.unshift(rootpath); + } - // also compile rtl-version if requested - let oRTL; - if (options.rtl) { - const RTLPlugin = require("./plugin/rtl"); - oRTL = new RTLPlugin(); - - const urls = oUrlCollector.getUrls(); - - const existingImgRtlUrls = (await Promise.all( - urls.map(async ({currentDirectory, relativeUrl}) => { - const relativeImgRtlUrl = RTLPlugin.getRtlImgUrl(relativeUrl); - if (relativeImgRtlUrl) { - const resolvedImgRtlUrl = path.posix.join(currentDirectory, relativeImgRtlUrl); - if (await that.fileUtils.findFile(resolvedImgRtlUrl, options.rootPaths)) { - return resolvedImgRtlUrl; - } - } - }) - )).filter(Boolean); + // also compile css-variables version if requested + if (options.cssVariables) { + // parse the content again to have a clean tree + const cssVariablesSkeletonTree = await parseContent(config.content); - oRTL.setExistingImgRtlPaths(existingImgRtlUrls); - } + // generate the skeleton-css and the less-variables + const CSSVariablesCollectorPlugin = require("./plugin/css-variables-collector"); + const oCSSVariablesCollector = new CSSVariablesCollectorPlugin(config); + const oVariableCollector = new VariableCollectorPlugin(options.compiler); + result.cssSkeleton = cssVariablesSkeletonTree.toCSS(Object.assign({}, options.compiler, { + plugins: [oCSSVariablesCollector, oVariableCollector] + })); + const varsOverride = oVariableCollector.getAllVariables(); + result.cssVariablesSource = oCSSVariablesCollector.toLessVariables(varsOverride); if (oRTL) { - result.cssRtl = tree.toCSS(Object.assign({}, options.compiler, { - plugins: [oRTL] + const oCSSVariablesCollectorRTL = new CSSVariablesCollectorPlugin(config); + result.cssSkeletonRtl = cssVariablesSkeletonTree.toCSS(Object.assign({}, options.compiler, { + plugins: [oCSSVariablesCollectorRTL, oRTL] })); } - if (rootpath) { - result.imports.unshift(rootpath); - } + // generate the css-variables content out of the less-variables + const cssVariablesTree = await parseContent(result.cssVariablesSource); + const CSSVariablesPointerPlugin = require("./plugin/css-variables-pointer"); + result.cssVariables = cssVariablesTree.toCSS(Object.assign({}, options.compiler, { + plugins: [new CSSVariablesPointerPlugin()] + })); + } - // also compile css-variables version if requested - if (options.cssVariables) { - return new Promise(function(resolve, reject) { - // parse the content again to have a clean tree - parser.parse(config.content, function(err, tree) { - if (err) { - reject(err); - } else { - resolve(tree); - } - }); - }).then(function(tree) { - // generate the skeleton-css and the less-variables - const CSSVariablesCollectorPlugin = require("./plugin/css-variables-collector"); - const oCSSVariablesCollector = new CSSVariablesCollectorPlugin(config); - const oVariableCollector = new VariableCollectorPlugin(options.compiler); - result.cssSkeleton = tree.toCSS(Object.assign({}, options.compiler, { - plugins: [oCSSVariablesCollector, oVariableCollector] - })); - const varsOverride = oVariableCollector.getAllVariables(); - result.cssVariablesSource = oCSSVariablesCollector.toLessVariables(varsOverride); - if (oRTL) { - const oCSSVariablesCollectorRTL = new CSSVariablesCollectorPlugin(config); - result.cssSkeletonRtl = tree.toCSS(Object.assign({}, options.compiler, { - plugins: [oCSSVariablesCollectorRTL, oRTL] - })); - } - return tree; - }).then(function(tree) { - // generate the css-variables content out of the less-variables - return new Promise(function(resolve, reject) { - parser.parse(result.cssVariablesSource, function(err, tree) { - if (err) { - reject(err); - } else { - const CSSVariablesPointerPlugin = require("./plugin/css-variables-pointer"); - result.cssVariables = tree.toCSS(Object.assign({}, options.compiler, { - plugins: [new CSSVariablesPointerPlugin()] - })); - resolve(result); - } - }); - }); - }); - } + return result; + } + async function addInlineParameters(result) { + // Inline parameters can only be added when the library name is known + if (typeof options.library !== "object" || typeof options.library.name !== "string") { return result; - }); - } + } - function addInlineParameters(result) { - return new Promise(function(resolve, reject) { - if (typeof options.library === "object" && typeof options.library.name === "string") { - const parameters = JSON.stringify(result.variables); + const parameters = JSON.stringify(result.variables); - // properly escape the parameters to be part of a data-uri - // + escaping single quote (') as it is used to surround the data-uri: url('...') - const escapedParameters = encodeURIComponent(parameters).replace(/'/g, function(char) { - return escape(char); - }); + // properly escape the parameters to be part of a data-uri + // + escaping single quote (') as it is used to surround the data-uri: url('...') + const escapedParameters = encodeURIComponent(parameters).replace(/'/g, function(char) { + return escape(char); + }); - // embed parameter variables as plain-text string into css - const parameterStyleRule = "\n/* Inline theming parameters */\n#sap-ui-theme-" + - options.library.name.replace(/\./g, "\\.") + - "{background-image:url('data:text/plain;utf-8," + escapedParameters + "')}\n"; + // embed parameter variables as plain-text string into css + const parameterStyleRule = "\n/* Inline theming parameters */\n#sap-ui-theme-" + + options.library.name.replace(/\./g, "\\.") + + "{background-image:url('data:text/plain;utf-8," + escapedParameters + "')}\n"; - // embed parameter variables as plain-text string into css - result.css += parameterStyleRule; - if (options.rtl) { - result.cssRtl += parameterStyleRule; - } - if (options.cssVariables) { - // for the css variables build we just add it to the variables - result.cssVariables += parameterStyleRule; - } - } - resolve(result); - }); + // embed parameter variables as plain-text string into css + result.css += parameterStyleRule; + if (options.rtl) { + result.cssRtl += parameterStyleRule; + } + if (options.cssVariables) { + // for the css variables build we just add it to the variables + result.cssVariables += parameterStyleRule; + } + + return result; } function getScopeVariables(options) { diff --git a/lib/plugin/css-variables-collector.js b/lib/plugin/css-variables-collector.js index 6b9fdc35..7380409a 100644 --- a/lib/plugin/css-variables-collector.js +++ b/lib/plugin/css-variables-collector.js @@ -58,20 +58,24 @@ CSSVariablesCollectorPlugin.prototype = { }); let lessVariables = ""; Object.keys(vars).forEach((value, index) => { - lessVariables += "@" + value + ": " + vars[value].css + ";\n"; + const variableName = value; + const variableValue = vars[value].css; + lessVariables += `@${variableName}: ${variableValue};\n`; }); Object.keys(this.calcVars).forEach((value, index) => { - lessVariables += "@" + value + ": " + this.calcVars[value].css + ";\n"; + const variableName = value; + const variableValue = this.calcVars[value].css; + lessVariables += `@${variableName}: ${variableValue};\n`; }); lessVariables += "\n:root {\n"; Object.keys(vars).forEach((value, index) => { if (vars[value].export) { - lessVariables += "--" + value + ": @" + value + ";\n"; + lessVariables += `--${value}: @${value};\n`; } }); Object.keys(this.calcVars).forEach((value, index) => { if (this.calcVars[value].export) { - lessVariables += "--" + value + ": @" + value + ";\n"; + lessVariables += `--${value}: @${value};\n`; } }); lessVariables += "}\n"; From cfc8bdc66dffb9dbddc62ea761f458d2319f4dcd Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Wed, 1 Sep 2021 11:22:03 +0200 Subject: [PATCH 2/8] [INTERNAL] Extract addInlineParameters from index.js Reducing the monolithic index.js script to improve testability of individual functionalities rather then just testing the whole build based on input/output. --- lib/index.js | 32 +----- lib/themingParameters/dataUri.js | 33 ++++++ package.json | 4 +- test/lib/themingParameters/dataUri.js | 159 ++++++++++++++++++++++++++ 4 files changed, 197 insertions(+), 31 deletions(-) create mode 100644 lib/themingParameters/dataUri.js create mode 100644 test/lib/themingParameters/dataUri.js diff --git a/lib/index.js b/lib/index.js index 1e639030..b2dc3458 100644 --- a/lib/index.js +++ b/lib/index.js @@ -10,6 +10,8 @@ const scope = require("./scope"); const fileUtilsFactory = require("./fileUtils"); +const themingParametersDataUri = require("./themingParameters/dataUri"); + // Plugins const ImportCollectorPlugin = require("./plugin/import-collector"); const VariableCollectorPlugin = require("./plugin/variable-collector"); @@ -311,35 +313,7 @@ Builder.prototype.build = function(options) { } async function addInlineParameters(result) { - // Inline parameters can only be added when the library name is known - if (typeof options.library !== "object" || typeof options.library.name !== "string") { - return result; - } - - const parameters = JSON.stringify(result.variables); - - // properly escape the parameters to be part of a data-uri - // + escaping single quote (') as it is used to surround the data-uri: url('...') - const escapedParameters = encodeURIComponent(parameters).replace(/'/g, function(char) { - return escape(char); - }); - - // embed parameter variables as plain-text string into css - const parameterStyleRule = "\n/* Inline theming parameters */\n#sap-ui-theme-" + - options.library.name.replace(/\./g, "\\.") + - "{background-image:url('data:text/plain;utf-8," + escapedParameters + "')}\n"; - - // embed parameter variables as plain-text string into css - result.css += parameterStyleRule; - if (options.rtl) { - result.cssRtl += parameterStyleRule; - } - if (options.cssVariables) { - // for the css variables build we just add it to the variables - result.cssVariables += parameterStyleRule; - } - - return result; + return themingParametersDataUri.addInlineParameters({result, options}); } function getScopeVariables(options) { diff --git a/lib/themingParameters/dataUri.js b/lib/themingParameters/dataUri.js new file mode 100644 index 00000000..fcbf1ecf --- /dev/null +++ b/lib/themingParameters/dataUri.js @@ -0,0 +1,33 @@ +module.exports = { + addInlineParameters: function({result, options}) { + // Inline parameters can only be added when the library name is known + if (typeof options.library !== "object" || typeof options.library.name !== "string") { + return result; + } + + const parameters = JSON.stringify(result.variables); + + // properly escape the parameters to be part of a data-uri + // + escaping single quote (') as it is used to surround the data-uri: url('...') + const escapedParameters = encodeURIComponent(parameters).replace(/'/g, function(char) { + return escape(char); + }); + + // embed parameter variables as plain-text string into css + const parameterStyleRule = "\n/* Inline theming parameters */\n#sap-ui-theme-" + + options.library.name.replace(/\./g, "\\.") + + "{background-image:url('data:text/plain;utf-8," + escapedParameters + "')}\n"; + + // embed parameter variables as plain-text string into css + result.css += parameterStyleRule; + if (options.rtl) { + result.cssRtl += parameterStyleRule; + } + if (options.cssVariables) { + // for the css variables build we just add it to the variables + result.cssVariables += parameterStyleRule; + } + + return result; + } +}; diff --git a/package.json b/package.json index 58c27666..85f889da 100644 --- a/package.json +++ b/package.json @@ -23,8 +23,8 @@ }, "scripts": { "lint": "eslint ./", - "unit": "mocha test/*.js", - "unit-debug": "mocha --inspect --inspect-brk test/*.js", + "unit": "mocha test/*.js test/lib/**/*.js", + "unit-debug": "mocha --inspect --inspect-brk test/*.js test/lib/**/*.js", "coverage": "nyc npm run unit", "test": "npm run lint && npm run coverage && npm run depcheck", "preversion": "npm test", diff --git a/test/lib/themingParameters/dataUri.js b/test/lib/themingParameters/dataUri.js new file mode 100644 index 00000000..21abd97d --- /dev/null +++ b/test/lib/themingParameters/dataUri.js @@ -0,0 +1,159 @@ +/* eslint-env mocha */ + +const assert = require("assert"); +// const sinon = require("sinon"); + +// tested module +const themingParametersDataUri = require("../../../lib/themingParameters/dataUri"); + +describe("themingParameters/dataUri", function() { + it("should not add theming parameters when library name is missing", function() { + const result = {}; + const options = {}; + const returnedResult = themingParametersDataUri.addInlineParameters({result, options}); + + assert.equal(returnedResult, result, "result object reference should be returned"); + assert.deepEqual(result, {}, "result object should not be modified"); + }); + + it("should add theming parameters to css", function() { + const result = { + css: "/* css */", + variables: {foo: "bar"} + }; + const options = { + library: { + name: "sap.ui.test" + } + }; + const returnedResult = themingParametersDataUri.addInlineParameters({result, options}); + + assert.equal(returnedResult, result, "result object reference should be returned"); + assert.deepEqual(result, { + variables: {foo: "bar"}, + css: `/* css */ +/* Inline theming parameters */ +#sap-ui-theme-sap\\.ui\\.test{background-image:url('data:text/plain;utf-8,%7B%22foo%22%3A%22bar%22%7D')} +` + }, "result.css should be enhanced"); + }); + + it("should encode variables to be included as data-uri", function() { + const result = { + css: "/* css */", + variables: { + foo: "50%", + bar: "'\"'" + } + }; + const options = { + library: { + name: "sap.ui.test" + } + }; + const returnedResult = themingParametersDataUri.addInlineParameters({result, options}); + + assert.equal(returnedResult, result, "result object reference should be returned"); + assert.deepEqual(result, { + variables: { + foo: "50%", + bar: "'\"'" + }, + css: `/* css */ +/* Inline theming parameters */ +#sap-ui-theme-sap\\.ui\\.test{background-image:url('\ +data:text/plain;utf-8,%7B%22foo%22%3A%2250%25%22%2C%22bar%22%3A%22%27%5C%22%27%22%7D')} +` + }, "result.css should be enhanced"); + }); + + it("should add theming parameters to cssRtl when options.rtl=true", function() { + const result = { + css: "/* css */", + cssRtl: "/* cssRtl */", + variables: {foo: "bar"} + }; + const options = { + library: { + name: "sap.ui.test" + }, + rtl: true + }; + const returnedResult = themingParametersDataUri.addInlineParameters({result, options}); + + assert.equal(returnedResult, result, "result object reference should be returned"); + assert.deepEqual(result, { + variables: {foo: "bar"}, + css: `/* css */ +/* Inline theming parameters */ +#sap-ui-theme-sap\\.ui\\.test{background-image:url('data:text/plain;utf-8,%7B%22foo%22%3A%22bar%22%7D')} +`, + cssRtl: `/* cssRtl */ +/* Inline theming parameters */ +#sap-ui-theme-sap\\.ui\\.test{background-image:url('data:text/plain;utf-8,%7B%22foo%22%3A%22bar%22%7D')} +` + }, "result.css should be enhanced"); + }); + + it("should add theming parameters to cssVariables when options.cssVariables=true", function() { + const result = { + css: "/* css */", + cssVariables: "/* cssVariables */", + variables: {foo: "bar"} + }; + const options = { + library: { + name: "sap.ui.test" + }, + cssVariables: true + }; + const returnedResult = themingParametersDataUri.addInlineParameters({result, options}); + + assert.equal(returnedResult, result, "result object reference should be returned"); + assert.deepEqual(result, { + variables: {foo: "bar"}, + css: `/* css */ +/* Inline theming parameters */ +#sap-ui-theme-sap\\.ui\\.test{background-image:url('data:text/plain;utf-8,%7B%22foo%22%3A%22bar%22%7D')} +`, + cssVariables: `/* cssVariables */ +/* Inline theming parameters */ +#sap-ui-theme-sap\\.ui\\.test{background-image:url('data:text/plain;utf-8,%7B%22foo%22%3A%22bar%22%7D')} +` + }, "result.css should be enhanced"); + }); + + it("should add theming parameters to cssRtl / cssVariables when options.rtl/cssVariables=true", function() { + const result = { + css: "/* css */", + cssRtl: "/* cssRtl */", + cssVariables: "/* cssVariables */", + variables: {foo: "bar"} + }; + const options = { + library: { + name: "sap.ui.test" + }, + rtl: true, + cssVariables: true + }; + const returnedResult = themingParametersDataUri.addInlineParameters({result, options}); + + assert.equal(returnedResult, result, "result object reference should be returned"); + assert.deepEqual(result, { + variables: {foo: "bar"}, + css: `/* css */ +/* Inline theming parameters */ +#sap-ui-theme-sap\\.ui\\.test{background-image:url('data:text/plain;utf-8,%7B%22foo%22%3A%22bar%22%7D')} +`, + cssRtl: `/* cssRtl */ +/* Inline theming parameters */ +#sap-ui-theme-sap\\.ui\\.test{background-image:url('data:text/plain;utf-8,%7B%22foo%22%3A%22bar%22%7D')} +`, + cssVariables: `/* cssVariables */ +/* Inline theming parameters */ +#sap-ui-theme-sap\\.ui\\.test{background-image:url('data:text/plain;utf-8,%7B%22foo%22%3A%22bar%22%7D')} +` + }, "result.css should be enhanced"); + }); +}); From 710a8318c7c1e9428ac42b1b944541e2c8a736c2 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Wed, 1 Sep 2021 15:31:56 +0200 Subject: [PATCH 3/8] [INTERNAL] Extract compile from index.js --- lib/Compiler.js | 191 +++++++++++++++++++++++++++++++++++++++++++++++ lib/index.js | 195 ++++-------------------------------------------- 2 files changed, 205 insertions(+), 181 deletions(-) create mode 100644 lib/Compiler.js diff --git a/lib/Compiler.js b/lib/Compiler.js new file mode 100644 index 00000000..5bdc8b5a --- /dev/null +++ b/lib/Compiler.js @@ -0,0 +1,191 @@ +const path = require("path"); +const createParser = require("./less/parser"); +const clone = require("clone"); + +// Plugins +const ImportCollectorPlugin = require("./plugin/import-collector"); +const VariableCollectorPlugin = require("./plugin/variable-collector"); +const UrlCollector = require("./plugin/url-collector"); + +class Compiler { + constructor({options, fileUtils, customFs}) { + this.options = options; + this.fileUtils = fileUtils; + + // Stores mapping between "virtual" paths (used within LESS) and real filesystem paths. + // Only relevant when using the "rootPaths" option. + this.mFileMappings = {}; + + // Only use custom file handler when "rootPaths" or custom "fs" are used + if ((this.options.rootPaths && this.options.rootPaths.length > 0) || customFs) { + this.fnFileHandler = this.createFileHandler(); + } else { + this.fnFileHandler = undefined; + } + } + createFileHandler() { + return async (file, currentFileInfo, handleDataAndCallCallback, callback) => { + try { + let pathname; + + // support absolute paths such as "/resources/my/base.less" + if (path.posix.isAbsolute(file)) { + pathname = path.posix.normalize(file); + } else { + pathname = path.posix.join(currentFileInfo.currentDirectory, file); + } + + const result = await this.fileUtils.readFile(pathname, this.options.rootPaths); + if (!result) { + // eslint-disable-next-line no-console + console.log("File not found: " + pathname); + callback({type: "File", message: "Could not find file at path '" + pathname + "'"}); + } else { + try { + // Save import mapping to calculate full import paths later on + this.mFileMappings[currentFileInfo.rootFilename] = + this.mFileMappings[currentFileInfo.rootFilename] || {}; + this.mFileMappings[currentFileInfo.rootFilename][pathname] = result.path; + + handleDataAndCallCallback(pathname, result.content); + } catch (e) { + // eslint-disable-next-line no-console + console.log(e); + callback(e); + } + } + } catch (err) { + // eslint-disable-next-line no-console + console.log(err); + callback(err); + } + }; + } + async compile(config) { + const parserOptions = clone(this.options.parser); + let rootpath; + + if (config.path) { + // Keep rootpath for later usage in the ImportCollectorPlugin + rootpath = config.path; + parserOptions.filename = config.localPath; + } + + // inject the library name as prefix (e.g. "sap.m" as "_sap_m") + if (this.options.library && typeof this.options.library.name === "string") { + const libName = config.libName = this.options.library.name; + config.libPath = libName.replace(/\./g, "/"); + config.prefix = "_" + libName.replace(/\./g, "_") + "_"; + } else { + config.prefix = ""; // no library, no prefix + } + + // Keep filename for later usage (see ImportCollectorPlugin) as less modifies the parserOptions.filename + const filename = parserOptions.filename; + + const parser = createParser(parserOptions, this.fnFileHandler); + + function parseContent(content) { + return new Promise(function(resolve, reject) { + parser.parse(content, function(err, tree) { + if (err) { + reject(err); + } else { + resolve(tree); + } + }); + }); + } + + const tree = await parseContent(config.content); + const result = {tree}; + + // plugins to collect imported files and variable values + const oImportCollector = new ImportCollectorPlugin({ + importMappings: this.mFileMappings[filename] + }); + const oVariableCollector = new VariableCollectorPlugin(this.options.compiler); + const oUrlCollector = new UrlCollector(); + + // render to css + result.css = tree.toCSS(Object.assign({}, this.options.compiler, { + plugins: [oImportCollector, oVariableCollector, oUrlCollector] + })); + + // retrieve imported files + result.imports = oImportCollector.getImports(); + + // retrieve reduced set of variables + result.variables = oVariableCollector.getVariables(Object.keys(this.mFileMappings[filename] || {})); + + // retrieve all variables + result.allVariables = oVariableCollector.getAllVariables(); + + // also compile rtl-version if requested + let oRTL; + if (this.options.rtl) { + const RTLPlugin = require("./plugin/rtl"); + oRTL = new RTLPlugin(); + + const urls = oUrlCollector.getUrls(); + + const existingImgRtlUrls = (await Promise.all( + urls.map(async ({currentDirectory, relativeUrl}) => { + const relativeImgRtlUrl = RTLPlugin.getRtlImgUrl(relativeUrl); + if (relativeImgRtlUrl) { + const resolvedImgRtlUrl = path.posix.join(currentDirectory, relativeImgRtlUrl); + if (await this.fileUtils.findFile(resolvedImgRtlUrl, this.options.rootPaths)) { + return resolvedImgRtlUrl; + } + } + }) + )).filter(Boolean); + + oRTL.setExistingImgRtlPaths(existingImgRtlUrls); + } + + if (oRTL) { + result.cssRtl = tree.toCSS(Object.assign({}, this.options.compiler, { + plugins: [oRTL] + })); + } + + if (rootpath) { + result.imports.unshift(rootpath); + } + + // also compile css-variables version if requested + if (this.options.cssVariables) { + // parse the content again to have a clean tree + const cssVariablesSkeletonTree = await parseContent(config.content); + + // generate the skeleton-css and the less-variables + const CSSVariablesCollectorPlugin = require("./plugin/css-variables-collector"); + const oCSSVariablesCollector = new CSSVariablesCollectorPlugin(config); + const oVariableCollector = new VariableCollectorPlugin(this.options.compiler); + result.cssSkeleton = cssVariablesSkeletonTree.toCSS(Object.assign({}, this.options.compiler, { + plugins: [oCSSVariablesCollector, oVariableCollector] + })); + const varsOverride = oVariableCollector.getAllVariables(); + result.cssVariablesSource = oCSSVariablesCollector.toLessVariables(varsOverride); + + if (oRTL) { + const oCSSVariablesCollectorRTL = new CSSVariablesCollectorPlugin(config); + result.cssSkeletonRtl = cssVariablesSkeletonTree.toCSS(Object.assign({}, this.options.compiler, { + plugins: [oCSSVariablesCollectorRTL, oRTL] + })); + } + + // generate the css-variables content out of the less-variables + const cssVariablesTree = await parseContent(result.cssVariablesSource); + const CSSVariablesPointerPlugin = require("./plugin/css-variables-pointer"); + result.cssVariables = cssVariablesTree.toCSS(Object.assign({}, this.options.compiler, { + plugins: [new CSSVariablesPointerPlugin()] + })); + } + + return result; + } +} + +module.exports = Compiler; diff --git a/lib/index.js b/lib/index.js index b2dc3458..217bd5a2 100644 --- a/lib/index.js +++ b/lib/index.js @@ -1,21 +1,17 @@ "use strict"; const path = require("path"); -const clone = require("clone"); const css = require("css"); -const createParser = require("./less/parser"); + const diff = require("./diff"); const scope = require("./scope"); const fileUtilsFactory = require("./fileUtils"); -const themingParametersDataUri = require("./themingParameters/dataUri"); +const Compiler = require("./Compiler"); -// Plugins -const ImportCollectorPlugin = require("./plugin/import-collector"); -const VariableCollectorPlugin = require("./plugin/variable-collector"); -const UrlCollector = require("./plugin/url-collector"); +const themingParametersDataUri = require("./themingParameters/dataUri"); // Workaround for a performance issue in the "css" parser module when used in combination // with the "colors" module that enhances the String prototype. @@ -117,10 +113,6 @@ Builder.prototype.cacheTheme = function(result) { Builder.prototype.build = function(options) { const that = this; - // Stores mapping between "virtual" paths (used within LESS) and real filesystem paths. - // Only relevant when using the "rootPaths" option. - const mFileMappings = {}; - // Assign default options options = Object.assign({ lessInput: null, @@ -145,172 +137,11 @@ Builder.prototype.build = function(options) { options.parser.relativeUrls = true; } - function fileHandler(file, currentFileInfo, handleDataAndCallCallback, callback) { - let pathname; - - // support absolute paths such as "/resources/my/base.less" - if (path.posix.isAbsolute(file)) { - pathname = path.posix.normalize(file); - } else { - pathname = path.posix.join(currentFileInfo.currentDirectory, file); - } - - that.fileUtils.readFile(pathname, options.rootPaths).then(function(result) { - if (!result) { - // eslint-disable-next-line no-console - console.log("File not found: " + pathname); - callback({type: "File", message: "Could not find file at path '" + pathname + "'"}); - } else { - try { - // Save import mapping to calculate full import paths later on - mFileMappings[currentFileInfo.rootFilename] = mFileMappings[currentFileInfo.rootFilename] || {}; - mFileMappings[currentFileInfo.rootFilename][pathname] = result.path; - - handleDataAndCallCallback(pathname, result.content); - } catch (e) { - // eslint-disable-next-line no-console - console.log(e); - callback(e); - } - } - }, function(err) { - // eslint-disable-next-line no-console - console.log(err); - callback(err); - }); - } - - async function compile(config) { - const parserOptions = clone(options.parser); - let rootpath; - - if (config.path) { - // Keep rootpath for later usage in the ImportCollectorPlugin - rootpath = config.path; - parserOptions.filename = config.localPath; - } - - // inject the library name as prefix (e.g. "sap.m" as "_sap_m") - if (options.library && typeof options.library.name === "string") { - const libName = config.libName = options.library.name; - config.libPath = libName.replace(/\./g, "/"); - config.prefix = "_" + libName.replace(/\./g, "_") + "_"; - } else { - config.prefix = ""; // no library, no prefix - } - - // Keep filename for later usage (see ImportCollectorPlugin) as less modifies the parserOptions.filename - const filename = parserOptions.filename; - - // Only use custom file handler when "rootPaths" or custom "fs" are used - let fnFileHandler; - if ((options.rootPaths && options.rootPaths.length > 0) || that.customFs) { - fnFileHandler = fileHandler; - } - - const parser = createParser(parserOptions, fnFileHandler); - - function parseContent(content) { - return new Promise(function(resolve, reject) { - parser.parse(content, function(err, tree) { - if (err) { - reject(err); - } else { - resolve(tree); - } - }); - }); - } - - const tree = await parseContent(config.content); - const result = {tree}; - - // plugins to collect imported files and variable values - const oImportCollector = new ImportCollectorPlugin({ - importMappings: mFileMappings[filename] - }); - const oVariableCollector = new VariableCollectorPlugin(options.compiler); - const oUrlCollector = new UrlCollector(); - - // render to css - result.css = tree.toCSS(Object.assign({}, options.compiler, { - plugins: [oImportCollector, oVariableCollector, oUrlCollector] - })); - - // retrieve imported files - result.imports = oImportCollector.getImports(); - - // retrieve reduced set of variables - result.variables = oVariableCollector.getVariables(Object.keys(mFileMappings[filename] || {})); - - // retrieve all variables - result.allVariables = oVariableCollector.getAllVariables(); - - // also compile rtl-version if requested - let oRTL; - if (options.rtl) { - const RTLPlugin = require("./plugin/rtl"); - oRTL = new RTLPlugin(); - - const urls = oUrlCollector.getUrls(); - - const existingImgRtlUrls = (await Promise.all( - urls.map(async ({currentDirectory, relativeUrl}) => { - const relativeImgRtlUrl = RTLPlugin.getRtlImgUrl(relativeUrl); - if (relativeImgRtlUrl) { - const resolvedImgRtlUrl = path.posix.join(currentDirectory, relativeImgRtlUrl); - if (await that.fileUtils.findFile(resolvedImgRtlUrl, options.rootPaths)) { - return resolvedImgRtlUrl; - } - } - }) - )).filter(Boolean); - - oRTL.setExistingImgRtlPaths(existingImgRtlUrls); - } - - if (oRTL) { - result.cssRtl = tree.toCSS(Object.assign({}, options.compiler, { - plugins: [oRTL] - })); - } - - if (rootpath) { - result.imports.unshift(rootpath); - } - - // also compile css-variables version if requested - if (options.cssVariables) { - // parse the content again to have a clean tree - const cssVariablesSkeletonTree = await parseContent(config.content); - - // generate the skeleton-css and the less-variables - const CSSVariablesCollectorPlugin = require("./plugin/css-variables-collector"); - const oCSSVariablesCollector = new CSSVariablesCollectorPlugin(config); - const oVariableCollector = new VariableCollectorPlugin(options.compiler); - result.cssSkeleton = cssVariablesSkeletonTree.toCSS(Object.assign({}, options.compiler, { - plugins: [oCSSVariablesCollector, oVariableCollector] - })); - const varsOverride = oVariableCollector.getAllVariables(); - result.cssVariablesSource = oCSSVariablesCollector.toLessVariables(varsOverride); - - if (oRTL) { - const oCSSVariablesCollectorRTL = new CSSVariablesCollectorPlugin(config); - result.cssSkeletonRtl = cssVariablesSkeletonTree.toCSS(Object.assign({}, options.compiler, { - plugins: [oCSSVariablesCollectorRTL, oRTL] - })); - } - - // generate the css-variables content out of the less-variables - const cssVariablesTree = await parseContent(result.cssVariablesSource); - const CSSVariablesPointerPlugin = require("./plugin/css-variables-pointer"); - result.cssVariables = cssVariablesTree.toCSS(Object.assign({}, options.compiler, { - plugins: [new CSSVariablesPointerPlugin()] - })); - } - - return result; - } + const compiler = new Compiler({ + options, + fileUtils: this.fileUtils, + customFs: this.customFs + }); async function addInlineParameters(result) { return themingParametersDataUri.addInlineParameters({result, options}); @@ -346,13 +177,13 @@ Builder.prototype.build = function(options) { if (!config) { throw new Error("Could not find embeddedCompareFile at path '" + scopeOptions.embeddedCompareFilePath + "'"); } - return compile(config); + return compiler.compile(config); }), that.fileUtils.readFile(scopeOptions.embeddedFilePath, options.rootPaths).then(function(config) { if (!config) { throw new Error("Could not find embeddedFile at path '" + scopeOptions.embeddedFilePath + "'"); } - return compile(config); + return compiler.compile(config); }) ]).then(function(results) { return { @@ -494,7 +325,9 @@ Builder.prototype.build = function(options) { } // No css diffing and scoping - return that.fileUtils.readFile(options.lessInputPath, options.rootPaths).then(compile); + return that.fileUtils.readFile(options.lessInputPath, options.rootPaths).then((config) => { + return compiler.compile(config); + }); }); } @@ -507,7 +340,7 @@ Builder.prototype.build = function(options) { } if (options.lessInput) { - return compile({ + return compiler.compile({ content: options.lessInput }).then(addInlineParameters).then(that.cacheTheme.bind(that)); } else { From 8575252f2f60332a90495c8735c2570d67a195dc Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Wed, 1 Sep 2021 16:06:04 +0200 Subject: [PATCH 4/8] [INTERNAL] Add missing tests for Compiler.js --- lib/Compiler.js | 18 ++--- package.json | 4 +- test/lib/Compiler.js | 97 +++++++++++++++++++++++++++ test/lib/themingParameters/dataUri.js | 1 - 4 files changed, 105 insertions(+), 15 deletions(-) create mode 100644 test/lib/Compiler.js diff --git a/lib/Compiler.js b/lib/Compiler.js index 5bdc8b5a..c68316b3 100644 --- a/lib/Compiler.js +++ b/lib/Compiler.js @@ -41,18 +41,12 @@ class Compiler { console.log("File not found: " + pathname); callback({type: "File", message: "Could not find file at path '" + pathname + "'"}); } else { - try { - // Save import mapping to calculate full import paths later on - this.mFileMappings[currentFileInfo.rootFilename] = - this.mFileMappings[currentFileInfo.rootFilename] || {}; - this.mFileMappings[currentFileInfo.rootFilename][pathname] = result.path; - - handleDataAndCallCallback(pathname, result.content); - } catch (e) { - // eslint-disable-next-line no-console - console.log(e); - callback(e); - } + // Save import mapping to calculate full import paths later on + this.mFileMappings[currentFileInfo.rootFilename] = + this.mFileMappings[currentFileInfo.rootFilename] || {}; + this.mFileMappings[currentFileInfo.rootFilename][pathname] = result.path; + + handleDataAndCallCallback(pathname, result.content); } } catch (err) { // eslint-disable-next-line no-console diff --git a/package.json b/package.json index 85f889da..2a60264a 100644 --- a/package.json +++ b/package.json @@ -23,8 +23,8 @@ }, "scripts": { "lint": "eslint ./", - "unit": "mocha test/*.js test/lib/**/*.js", - "unit-debug": "mocha --inspect --inspect-brk test/*.js test/lib/**/*.js", + "unit": "mocha test/*.js test/lib/**", + "unit-debug": "mocha --inspect --inspect-brk test/*.js test/lib/**", "coverage": "nyc npm run unit", "test": "npm run lint && npm run coverage && npm run depcheck", "preversion": "npm test", diff --git a/test/lib/Compiler.js b/test/lib/Compiler.js new file mode 100644 index 00000000..4fa5d61f --- /dev/null +++ b/test/lib/Compiler.js @@ -0,0 +1,97 @@ +/* eslint-env mocha */ + +const assert = require("assert"); +const sinon = require("sinon"); + +// tested module +const Compiler = require("../../lib/Compiler"); + +describe("Compiler#createFileHandler", function() { + before(() => { + sinon.stub(console, "log"); + }); + after(() => { + sinon.restore(); + }); + + it("should propagate errors via callback function (TypeError)", async function() { + const compiler = new Compiler({ + options: { + rootPaths: ["foo"] + }, + fileUtils: {}, + }); + + const file = "someFile"; + const currentFileInfo = { + currentDirectory: undefined // This will cause a TypeError when calling path.join + }; + const handleDataAndCallCallback = sinon.stub(); + const callback = sinon.stub(); + + await compiler.fnFileHandler(file, currentFileInfo, handleDataAndCallCallback, callback); + + assert.equal(handleDataAndCallCallback.callCount, 0); + assert.equal(callback.callCount, 1); + assert.equal(callback.getCall(0).args.length, 1); + assert.equal(callback.getCall(0).args[0].name, "TypeError"); + assert.equal(callback.getCall(0).args[0].message, + `The "path" argument must be of type string. Received undefined` + ); + }); + it("should propagate errors via callback function (File not found)", async function() { + const compiler = new Compiler({ + options: { + rootPaths: ["foo"] + }, + fileUtils: { + readFile: sinon.stub().resolves(null) + }, + }); + + const file = "someFile"; + const currentFileInfo = { + currentDirectory: "someFolder" + }; + const handleDataAndCallCallback = sinon.stub(); + const callback = sinon.stub(); + + await compiler.fnFileHandler(file, currentFileInfo, handleDataAndCallCallback, callback); + + assert.equal(handleDataAndCallCallback.callCount, 0); + assert.equal(callback.callCount, 1); + assert.equal(callback.getCall(0).args.length, 1); + assert.equal(callback.getCall(0).args[0].type, "File"); + assert.equal(callback.getCall(0).args[0].message, + `Could not find file at path 'someFolder/someFile'` + ); + }); + it("should propagate errors via callback function (error within handleDataAndCallCallback)", async function() { + const compiler = new Compiler({ + options: { + rootPaths: ["foo"] + }, + fileUtils: { + readFile: sinon.stub().resolves({ + path: "", content: "" + }) + }, + }); + + const file = "someFile"; + const currentFileInfo = { + currentDirectory: "someFolder" + }; + const handleDataAndCallCallback = sinon.stub().throws(new Error("Error from handleDataAndCallCallback")); + const callback = sinon.stub(); + + await compiler.fnFileHandler(file, currentFileInfo, handleDataAndCallCallback, callback); + + assert.equal(handleDataAndCallCallback.callCount, 1); + assert.equal(callback.callCount, 1); + assert.equal(callback.getCall(0).args.length, 1); + assert.equal(callback.getCall(0).args[0].message, + `Error from handleDataAndCallCallback` + ); + }); +}); diff --git a/test/lib/themingParameters/dataUri.js b/test/lib/themingParameters/dataUri.js index 21abd97d..a7d64ea8 100644 --- a/test/lib/themingParameters/dataUri.js +++ b/test/lib/themingParameters/dataUri.js @@ -1,7 +1,6 @@ /* eslint-env mocha */ const assert = require("assert"); -// const sinon = require("sinon"); // tested module const themingParametersDataUri = require("../../../lib/themingParameters/dataUri"); From 9f83d71450624ed3b4ae447990bc5d47c5af4728 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Wed, 1 Sep 2021 16:06:36 +0200 Subject: [PATCH 5/8] [INTERNAL] Increase coverage threshold --- package.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index 2a60264a..ae19c199 100644 --- a/package.json +++ b/package.json @@ -52,10 +52,10 @@ "lib/thirdparty/**" ], "check-coverage": true, - "statements": 90, + "statements": 95, "branches": 85, - "functions": 90, - "lines": 90, + "functions": 100, + "lines": 95, "watermarks": { "statements": [ 70, From 01e420d4f314a92d07f31c9233df1edc26efd953 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Wed, 1 Sep 2021 16:17:36 +0200 Subject: [PATCH 6/8] [INTERNAL] Remove check for TypeError name Might be different in older Node.js versions --- test/lib/Compiler.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/lib/Compiler.js b/test/lib/Compiler.js index 4fa5d61f..46cf7ee3 100644 --- a/test/lib/Compiler.js +++ b/test/lib/Compiler.js @@ -34,7 +34,6 @@ describe("Compiler#createFileHandler", function() { assert.equal(handleDataAndCallCallback.callCount, 0); assert.equal(callback.callCount, 1); assert.equal(callback.getCall(0).args.length, 1); - assert.equal(callback.getCall(0).args[0].name, "TypeError"); assert.equal(callback.getCall(0).args[0].message, `The "path" argument must be of type string. Received undefined` ); From 59a73a821c3e8be7c1a77447d00ef552fcde92ff Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Wed, 1 Sep 2021 16:23:40 +0200 Subject: [PATCH 7/8] [INTERNAL] Fix TypeError test on Node v10 --- test/lib/Compiler.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/lib/Compiler.js b/test/lib/Compiler.js index 46cf7ee3..cac9886c 100644 --- a/test/lib/Compiler.js +++ b/test/lib/Compiler.js @@ -19,12 +19,12 @@ describe("Compiler#createFileHandler", function() { options: { rootPaths: ["foo"] }, - fileUtils: {}, + fileUtils: undefined // This will cause a TypeError when calling fileUtils.readFile }); const file = "someFile"; const currentFileInfo = { - currentDirectory: undefined // This will cause a TypeError when calling path.join + currentDirectory: "someFolder" }; const handleDataAndCallCallback = sinon.stub(); const callback = sinon.stub(); @@ -34,8 +34,9 @@ describe("Compiler#createFileHandler", function() { assert.equal(handleDataAndCallCallback.callCount, 0); assert.equal(callback.callCount, 1); assert.equal(callback.getCall(0).args.length, 1); + assert.equal(callback.getCall(0).args[0].name, "TypeError"); assert.equal(callback.getCall(0).args[0].message, - `The "path" argument must be of type string. Received undefined` + `Cannot read property 'readFile' of undefined` ); }); it("should propagate errors via callback function (File not found)", async function() { From 38faa1d1a19ae06b600282c2d188b3cbe9d4880e Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Thu, 2 Sep 2021 08:54:58 +0200 Subject: [PATCH 8/8] [INTERNAL] Minor changes based on code review --- lib/index.js | 2 +- lib/plugin/css-variables-collector.js | 37 +++++++++++---------------- 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/lib/index.js b/lib/index.js index 217bd5a2..0d7783a6 100644 --- a/lib/index.js +++ b/lib/index.js @@ -143,7 +143,7 @@ Builder.prototype.build = function(options) { customFs: this.customFs }); - async function addInlineParameters(result) { + function addInlineParameters(result) { return themingParametersDataUri.addInlineParameters({result, options}); } diff --git a/lib/plugin/css-variables-collector.js b/lib/plugin/css-variables-collector.js index 7380409a..458dec71 100644 --- a/lib/plugin/css-variables-collector.js +++ b/lib/plugin/css-variables-collector.js @@ -44,38 +44,31 @@ CSSVariablesCollectorPlugin.prototype = { toLessVariables(varsOverride) { const vars = {}; - Object.keys(this.vars).forEach((key) => { - const override = this.vars[key].updateAfterEval && varsOverride[key] !== undefined; - /* - if (override) { - console.log(`Override variable "${key}" from "${this.vars[key].css}" to "${varsOverride[key]}"`); - } - */ - vars[key] = { - css: override ? varsOverride[key] : this.vars[key].css, - export: this.vars[key].export + Object.keys(this.vars).forEach((variableName) => { + const override = this.vars[variableName].updateAfterEval && varsOverride[variableName] !== undefined; + vars[variableName] = { + css: override ? varsOverride[variableName] : this.vars[variableName].css, + export: this.vars[variableName].export }; }); let lessVariables = ""; - Object.keys(vars).forEach((value, index) => { - const variableName = value; - const variableValue = vars[value].css; + Object.keys(vars).forEach((variableName) => { + const variableValue = vars[variableName].css; lessVariables += `@${variableName}: ${variableValue};\n`; }); - Object.keys(this.calcVars).forEach((value, index) => { - const variableName = value; - const variableValue = this.calcVars[value].css; + Object.keys(this.calcVars).forEach((variableName) => { + const variableValue = this.calcVars[variableName].css; lessVariables += `@${variableName}: ${variableValue};\n`; }); lessVariables += "\n:root {\n"; - Object.keys(vars).forEach((value, index) => { - if (vars[value].export) { - lessVariables += `--${value}: @${value};\n`; + Object.keys(vars).forEach((variableName) => { + if (vars[variableName].export) { + lessVariables += `--${variableName}: @${variableName};\n`; } }); - Object.keys(this.calcVars).forEach((value, index) => { - if (this.calcVars[value].export) { - lessVariables += `--${value}: @${value};\n`; + Object.keys(this.calcVars).forEach((variableName) => { + if (this.calcVars[variableName].export) { + lessVariables += `--${variableName}: @${variableName};\n`; } }); lessVariables += "}\n";