Skip to content

Commit

Permalink
[BREAKING] libraryLessGenerator: Throw error when import can't be inl…
Browse files Browse the repository at this point in the history 
…ined

This partially reverts #571.

The processor now fails again when an import can't be inlined, as this
will cause issues when processing the library within the
SAP Theme Designer.
  • Loading branch information
@matz3
matz3 committed Jan 11, 2023
1 parent e9f113d commit d2be9bb
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 69 deletions.
34 changes: 4 additions & 30 deletions lib/processors/libraryLessGenerator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,6 @@ const IMPORT_PATTERN = /@import .*"(.*)";/g;
const BASE_LESS_PATTERN = /^\/resources\/sap\/ui\/core\/themes\/([^/]+)\/base\.less$/;
const GLOBAL_LESS_PATTERN = /^\/resources\/sap\/ui\/core\/themes\/([^/]+)\/global\.less$/;

class ImportError extends Error {
constructor(message) {
super();
this.name = "ImportError";
this.message = message;
}
}

class LibraryLessGenerator {
constructor({fs}) {
const readFile = promisify(fs.readFile);
Expand All @@ -42,17 +34,7 @@ class LibraryLessGenerator {
const replacements = await Promise.all(imports.map(async (importMatch) => {
const baseDir = posixPath.dirname(filePath);
const resolvedFilePath = posixPath.resolve(baseDir, importMatch.path);
try {
importMatch.content = await this.resolveLessImport(importMatch.path, resolvedFilePath, baseDir);
} catch (error) {
if (error instanceof ImportError) {
// Add message of import errors after the import statements
// Currently those errors should not break the build (see comments in resolveLessImport)
importMatch.content = importMatch.fullMatch + ` /* ${error} */`;
} else {
throw error;
}
}
importMatch.content = await this.resolveLessImport(importMatch.path, resolvedFilePath, baseDir);
return importMatch;
}));

Expand Down Expand Up @@ -97,26 +79,19 @@ class LibraryLessGenerator {
}

/*
* Log error in case of files which are not in the same directory as the current file because
* Throw error in case of files which are not in the same directory as the current file because
* inlining them would break relative URLs.
* A possible solution would be to rewrite relative URLs when inlining the content.
*
* Keeping the import will cause errors since only "library.less" and "global.less" are
* configured to be available to the Theme Designer (.theming generated in generateThemeDesignerResources).
* However, the previous implementation did not break the build.
* In many cases the library.less file is currently not relevant so breaking the build would cause
* unnecessary issues.
*
*/
const relativeFilePath = posixPath.relative(baseDir, resolvedFilePath);
if (relativeFilePath.includes(posixPath.sep)) {
log.error(
throw new Error(
`Could not inline import '${resolvedFilePath}' outside of theme directory '${baseDir}'. ` +
`Stylesheets must be located in the theme directory (no sub-directories). ` +
`The generated '${baseDir}/library.less' will cause errors when compiled with the Theme Designer.`
`Stylesheets must be located in the theme directory (no sub-directories).`
);
// Throw error to be added as comment to the import statement
throw new ImportError("Could not inline import outside of theme directory");
}

let importedFileContent;
Expand Down Expand Up @@ -145,7 +120,6 @@ class LibraryLessGenerator {
while ((match = IMPORT_PATTERN.exec(fileContent)) !== null) {
imports.push({
path: match[1],
fullMatch: match[0],
matchStart: match.index,
matchLength: match[0].length
});
Expand Down
60 changes: 21 additions & 39 deletions test/lib/processors/libraryLessGenerator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -420,12 +420,10 @@ test.serial("LibraryLessGenerator: Rewrite for library.source.less", async (t) =
t.is(loggerStub.error.callCount, 0);
});

test.serial("LibraryLessGenerator: loggerStub error for file in sub-directory", async (t) => {
test.serial("LibraryLessGenerator: Throw error for file in sub-directory", async (t) => {
const {LibraryLessGenerator, loggerStub} = t.context;

const input = `@import "foo/bar.less";`;
const expectedOutput = `${FILE_HEADER}\n\n` +
`@import "foo/bar.less"; /* ImportError: Could not inline import outside of theme directory */`;

const fs = {
readFile: sinon.stub().callsFake((filePath, options, cb) => {
Expand All @@ -435,29 +433,23 @@ test.serial("LibraryLessGenerator: loggerStub error for file in sub-directory",
})
};

const output = await (new LibraryLessGenerator({fs})).generate({
await t.throwsAsync(new LibraryLessGenerator({fs}).generate({
filePath: "/resources/sap/ui/core/themes/sap_fiori_3/library.source.less",
fileContent: input
}), {
message: `Could not inline import '/resources/sap/ui/core/themes/sap_fiori_3/foo/bar.less' ` +
`outside of theme directory '/resources/sap/ui/core/themes/sap_fiori_3'. ` +
`Stylesheets must be located in the theme directory (no sub-directories).`
});

t.is(output, expectedOutput);
t.is(fs.readFile.callCount, 0, "fs.readFile should not be called");
t.is(loggerStub.error.callCount, 1);
t.deepEqual(loggerStub.error.getCall(0).args, [
"Could not inline import '/resources/sap/ui/core/themes/sap_fiori_3/foo/bar.less' " +
"outside of theme directory '/resources/sap/ui/core/themes/sap_fiori_3'. " +
"Stylesheets must be located in the theme directory (no sub-directories). " +
"The generated '/resources/sap/ui/core/themes/sap_fiori_3/library.less' will cause " +
"errors when compiled with the Theme Designer."
]);
t.is(loggerStub.error.callCount, 0);
});

test.serial("LibraryLessGenerator: loggerStub error for file outside of theme directory", async (t) => {
test.serial("LibraryLessGenerator: Throw error for file outside of theme directory", async (t) => {
const {LibraryLessGenerator, loggerStub} = t.context;

const input = `@import "../foo/bar.less";`;
const expectedOutput = `${FILE_HEADER}\n\n` +
`@import "../foo/bar.less"; /* ImportError: Could not inline import outside of theme directory */`;

const fs = {
readFile: sinon.stub().callsFake((filePath, options, cb) => {
Expand All @@ -467,29 +459,23 @@ test.serial("LibraryLessGenerator: loggerStub error for file outside of theme di
})
};

const output = await (new LibraryLessGenerator({fs})).generate({
await t.throwsAsync(new LibraryLessGenerator({fs}).generate({
filePath: "/resources/sap/ui/core/themes/sap_fiori_3/library.source.less",
fileContent: input
}), {
message: `Could not inline import '/resources/sap/ui/core/themes/foo/bar.less' outside of theme directory ` +
`'/resources/sap/ui/core/themes/sap_fiori_3'. ` +
`Stylesheets must be located in the theme directory (no sub-directories).`
});

t.is(output, expectedOutput);
t.is(fs.readFile.callCount, 0, "fs.readFile should not be called");
t.is(loggerStub.error.callCount, 1);
t.deepEqual(loggerStub.error.getCall(0).args, [
"Could not inline import '/resources/sap/ui/core/themes/foo/bar.less' " +
"outside of theme directory '/resources/sap/ui/core/themes/sap_fiori_3'. " +
"Stylesheets must be located in the theme directory (no sub-directories). " +
"The generated '/resources/sap/ui/core/themes/sap_fiori_3/library.less' will cause " +
"errors when compiled with the Theme Designer."
]);
t.is(loggerStub.error.callCount, 0);
});

test.serial("LibraryLessGenerator: loggerStub error for absolute import outside of theme directory", async (t) => {
test.serial("LibraryLessGenerator: Throw error for absolute import outside of theme directory", async (t) => {
const {LibraryLessGenerator, loggerStub} = t.context;

const input = `@import "/foo/bar.less";`;
const expectedOutput = `${FILE_HEADER}\n\n` +
`@import "/foo/bar.less"; /* ImportError: Could not inline import outside of theme directory */`;

const fs = {
readFile: sinon.stub().callsFake((filePath, options, cb) => {
Expand All @@ -499,21 +485,17 @@ test.serial("LibraryLessGenerator: loggerStub error for absolute import outside
})
};

const output = await (new LibraryLessGenerator({fs})).generate({
await t.throwsAsync(new LibraryLessGenerator({fs}).generate({
filePath: "/resources/sap/ui/core/themes/sap_fiori_3/library.source.less",
fileContent: input
}), {
message: `Could not inline import '/foo/bar.less' outside of theme directory ` +
`'/resources/sap/ui/core/themes/sap_fiori_3'. ` +
`Stylesheets must be located in the theme directory (no sub-directories).`
});

t.is(output, expectedOutput);
t.is(fs.readFile.callCount, 0, "fs.readFile should not be called");
t.is(loggerStub.error.callCount, 1);
t.deepEqual(loggerStub.error.getCall(0).args, [
"Could not inline import '/foo/bar.less' " +
"outside of theme directory '/resources/sap/ui/core/themes/sap_fiori_3'. " +
"Stylesheets must be located in the theme directory (no sub-directories). " +
"The generated '/resources/sap/ui/core/themes/sap_fiori_3/library.less' will cause " +
"errors when compiled with the Theme Designer."
]);
t.is(loggerStub.error.callCount, 0);
});

test.serial("LibraryLessGenerator: Throw error when file can't be found", async (t) => {
Expand Down

0 comments on commit d2be9bb

Please sign in to comment.