Skip to content

Commit 5568cd6

Browse files
matz3matz3
authored
[FIX] Variables: Include variables defined in sub-directories (#160)
Fixes: #159
1 parent 23862a9 commit 5568cd6

File tree

11 files changed

+191
-17
lines changed

11 files changed

+191
-17
lines changed

‎lib/plugin/variable-collector.js

Lines changed: 47 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
"use strict";
2-
const path = require("path");
2+
const posixPath = require("path").posix;
33
const less = require("../thirdparty/less");
4+
const backslashRegExp = /\\/g;
5+
6+
function toPosix(p) {
7+
return p.replace(backslashRegExp, "/");
8+
}
49

510
const VariableCollector = module.exports = function(env) {
611
/* eslint-disable new-cap */
@@ -43,15 +48,52 @@ VariableCollector.prototype = {
4348
getVariables: function(aImports) {
4449
const mVariables = {};
4550

51+
const aPosixImports = aImports.map(toPosix);
52+
53+
const includeCache = {};
54+
55+
function shouldIncludeVariable({rootFilename, filename}) {
56+
// Simple case where variable is defined in root file
57+
if (rootFilename === filename) {
58+
return true;
59+
}
60+
61+
const cacheKey = rootFilename + "|" + filename;
62+
if (includeCache[cacheKey] !== undefined) {
63+
return includeCache[cacheKey];
64+
}
65+
66+
const dirname = posixPath.dirname(filename);
67+
const baseFileName = posixPath.basename(rootFilename); // usually library.source.less
68+
69+
function check(currentDirname) {
70+
const libraryBaseFile = posixPath.join(currentDirname, baseFileName);
71+
if (libraryBaseFile === rootFilename || aPosixImports.includes(libraryBaseFile)) {
72+
return true;
73+
} else {
74+
// Recursively check parent directories whether they contain an imported "base file"
75+
// This is only relevant when a theme contains sub-directories
76+
const parentDirname = posixPath.dirname(currentDirname);
77+
if (parentDirname === currentDirname) {
78+
return false; // We are at the root
79+
} else {
80+
return check(parentDirname);
81+
}
82+
}
83+
}
84+
85+
return includeCache[cacheKey] = check(dirname);
86+
}
87+
4688
for (const name in this.mVariables) {
4789
if (Object.prototype.hasOwnProperty.call(this.mVariables, name)) {
4890
const oVar = this.mVariables[name];
49-
const dirname = path.posix.dirname(oVar.filename);
50-
const baseFileName = path.posix.basename(oVar.rootFilename); // usually library.source.less
51-
const libraryBaseFile = path.posix.normalize(path.posix.join(dirname, baseFileName));
91+
// Ensure posix paths
92+
const rootFilename = toPosix(oVar.rootFilename);
93+
const filename = toPosix(oVar.filename);
5294

5395
// Only add variable if the corresponding library "base file" has been imported
54-
if (aImports.indexOf(libraryBaseFile) > -1 || libraryBaseFile === oVar.rootFilename) {
96+
if (shouldIncludeVariable({rootFilename, filename})) {
5597
mVariables[name] = oVar.value;
5698
}
5799
}

‎test/expected/libraries/lib3/my/other/ui/lib/themes/bar/library-RTL.css

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,4 @@
1010
}
1111

1212
/* Inline theming parameters */
13-
#sap-ui-theme-my\.other\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22default%22%3A%7B%22_my_other_ui_lib_MyControl_color1%22%3A%22%23ffffff%22%2C%22_my_other_ui_lib_MyControl_color2%22%3A%22%23008000%22%7D%2C%22scopes%22%3A%7B%22barContrast%22%3A%7B%22_my_other_ui_lib_MyControl_color1%22%3A%22%23000000%22%7D%7D%7D')}
13+
#sap-ui-theme-my\.other\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22default%22%3A%7B%22_my_other_ui_lib_MyControl_color1%22%3A%22%23ffffff%22%2C%22_my_other_ui_lib_MyOtherControl_color1%22%3A%22%23ffffff%22%2C%22_my_other_ui_lib_MyControl_color2%22%3A%22%23008000%22%7D%2C%22scopes%22%3A%7B%22barContrast%22%3A%7B%22_my_other_ui_lib_MyControl_color1%22%3A%22%23000000%22%2C%22_my_other_ui_lib_MyOtherControl_color1%22%3A%22%23000000%22%7D%7D%7D')}

‎test/expected/libraries/lib3/my/other/ui/lib/themes/bar/library.css

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,4 @@
1010
}
1111

1212
/* Inline theming parameters */
13-
#sap-ui-theme-my\.other\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22default%22%3A%7B%22_my_other_ui_lib_MyControl_color1%22%3A%22%23ffffff%22%2C%22_my_other_ui_lib_MyControl_color2%22%3A%22%23008000%22%7D%2C%22scopes%22%3A%7B%22barContrast%22%3A%7B%22_my_other_ui_lib_MyControl_color1%22%3A%22%23000000%22%7D%7D%7D')}
13+
#sap-ui-theme-my\.other\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22default%22%3A%7B%22_my_other_ui_lib_MyControl_color1%22%3A%22%23ffffff%22%2C%22_my_other_ui_lib_MyOtherControl_color1%22%3A%22%23ffffff%22%2C%22_my_other_ui_lib_MyControl_color2%22%3A%22%23008000%22%7D%2C%22scopes%22%3A%7B%22barContrast%22%3A%7B%22_my_other_ui_lib_MyControl_color1%22%3A%22%23000000%22%2C%22_my_other_ui_lib_MyOtherControl_color1%22%3A%22%23000000%22%7D%7D%7D')}

‎test/expected/libraries/lib3/my/other/ui/lib/themes/base/library-RTL.css

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,4 @@
66
}
77

88
/* Inline theming parameters */
9-
#sap-ui-theme-my\.other\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22_my_other_ui_lib_MyControl_color1%22%3A%22%23fefefe%22%7D')}
9+
#sap-ui-theme-my\.other\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22_my_other_ui_lib_MyControl_color1%22%3A%22%23fefefe%22%2C%22_my_other_ui_lib_MyOtherControl_color1%22%3A%22%23fefefe%22%7D')}

‎test/expected/libraries/lib3/my/other/ui/lib/themes/base/library.css

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,4 @@
66
}
77

88
/* Inline theming parameters */
9-
#sap-ui-theme-my\.other\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22_my_other_ui_lib_MyControl_color1%22%3A%22%23fefefe%22%7D')}
9+
#sap-ui-theme-my\.other\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22_my_other_ui_lib_MyControl_color1%22%3A%22%23fefefe%22%2C%22_my_other_ui_lib_MyOtherControl_color1%22%3A%22%23fefefe%22%7D')}

‎test/expected/libraries/lib3/my/other/ui/lib/themes/foo/library-RTL.css

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,4 @@
1010
}
1111

1212
/* Inline theming parameters */
13-
#sap-ui-theme-my\.other\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22default%22%3A%7B%22_my_other_ui_lib_MyControl_color1%22%3A%22%23ffffff%22%2C%22_my_other_ui_lib_MyControl_color2%22%3A%22%23008000%22%7D%2C%22scopes%22%3A%7B%22fooContrast%22%3A%7B%22_my_other_ui_lib_MyControl_color1%22%3A%22%23000000%22%7D%7D%7D')}
13+
#sap-ui-theme-my\.other\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22default%22%3A%7B%22_my_other_ui_lib_MyControl_color1%22%3A%22%23ffffff%22%2C%22_my_other_ui_lib_MyOtherControl_color1%22%3A%22%23ffffff%22%2C%22_my_other_ui_lib_MyControl_color2%22%3A%22%23008000%22%7D%2C%22scopes%22%3A%7B%22fooContrast%22%3A%7B%22_my_other_ui_lib_MyControl_color1%22%3A%22%23000000%22%2C%22_my_other_ui_lib_MyOtherControl_color1%22%3A%22%23000000%22%7D%7D%7D')}

‎test/expected/libraries/lib3/my/other/ui/lib/themes/foo/library.css

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,4 @@
1010
}
1111

1212
/* Inline theming parameters */
13-
#sap-ui-theme-my\.other\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22default%22%3A%7B%22_my_other_ui_lib_MyControl_color1%22%3A%22%23ffffff%22%2C%22_my_other_ui_lib_MyControl_color2%22%3A%22%23008000%22%7D%2C%22scopes%22%3A%7B%22fooContrast%22%3A%7B%22_my_other_ui_lib_MyControl_color1%22%3A%22%23000000%22%7D%7D%7D')}
13+
#sap-ui-theme-my\.other\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22default%22%3A%7B%22_my_other_ui_lib_MyControl_color1%22%3A%22%23ffffff%22%2C%22_my_other_ui_lib_MyOtherControl_color1%22%3A%22%23ffffff%22%2C%22_my_other_ui_lib_MyControl_color2%22%3A%22%23008000%22%7D%2C%22scopes%22%3A%7B%22fooContrast%22%3A%7B%22_my_other_ui_lib_MyControl_color1%22%3A%22%23000000%22%2C%22_my_other_ui_lib_MyOtherControl_color1%22%3A%22%23000000%22%7D%7D%7D')}

‎test/fixtures/libraries/lib3/my/other/ui/lib/themes/base/library.source.less

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
@import "../../../../../../my/ui/lib/themes/base/global.less";
22
@import "MyControl.less";
3+
@import "sub-directory/MyOtherControl.less";

‎test/fixtures/libraries/lib3/my/other/ui/lib/themes/base/sub-directory/MyOtherControl.less

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
@_my_other_ui_lib_MyOtherControl_color1: @color1;

‎test/test-variable-collector.js

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
/* eslint-env mocha */
2+
"use strict";
3+
4+
const assert = require("assert");
5+
const VariableCollector = require("../lib/plugin/variable-collector");
6+
7+
describe("VariableCollector", function() {
8+
it("should return relevant variables when calling 'getVariables'", function() {
9+
const env = {};
10+
const variableCollector = new VariableCollector(env);
11+
12+
variableCollector.mVariables = {
13+
"color1": {
14+
"value": "#ffffff",
15+
"filename": "my/ui/lib/themes/foo/global.less",
16+
"rootFilename": "my/other/ui/lib/themes/foo/library.source.less"
17+
},
18+
"_my_other_ui_lib_MyControl_color1": {
19+
"value": "#ffffff",
20+
"filename": "my/other/ui/lib/themes/foo/MyControl.less",
21+
"rootFilename": "my/other/ui/lib/themes/foo/library.source.less"
22+
},
23+
"_my_other_ui_lib_MyOtherControl_color1": {
24+
"value": "#ffffff",
25+
"filename": "my/other/ui/lib/themes/base/sub-directory/MyOtherControl.less",
26+
"rootFilename": "my/other/ui/lib/themes/foo/library.source.less"
27+
},
28+
"_my_other_ui_lib_MyControl_color2": {
29+
"value": "#008000",
30+
"filename": "my/other/ui/lib/themes/foo/MyControl.less",
31+
"rootFilename": "my/other/ui/lib/themes/foo/library.source.less"
32+
}
33+
};
34+
35+
const aImports = [
36+
"my/ui/lib/themes/foo/global.less",
37+
"my/other/ui/lib/themes/foo/MyControl.less",
38+
"my/other/ui/lib/themes/base/library.source.less",
39+
"my/ui/lib/themes/base/global.less",
40+
"my/other/ui/lib/themes/base/MyControl.less",
41+
"my/other/ui/lib/themes/base/sub-directory/MyOtherControl.less"
42+
];
43+
44+
const variables = variableCollector.getVariables(aImports);
45+
46+
assert.deepEqual(variables, {
47+
"_my_other_ui_lib_MyControl_color1": "#ffffff",
48+
"_my_other_ui_lib_MyOtherControl_color1": "#ffffff",
49+
"_my_other_ui_lib_MyControl_color2": "#008000"
50+
}, "relevant variables should be returned");
51+
});
52+
it("should return relevant variables when calling 'getVariables' (win32 paths)", function() {
53+
const env = {};
54+
const variableCollector = new VariableCollector(env);
55+
56+
variableCollector.mVariables = {
57+
"color1": {
58+
"value": "#ffffff",
59+
"filename": "my\\ui\\lib\\themes\\foo\\global.less",
60+
"rootFilename": "my\\other\\ui\\lib\\themes\\foo\\library.source.less"
61+
},
62+
"_my_other_ui_lib_MyControl_color1": {
63+
"value": "#ffffff",
64+
"filename": "my\\other\\ui\\lib\\themes\\foo\\MyControl.less",
65+
"rootFilename": "my\\other\\ui\\lib\\themes\\foo\\library.source.less"
66+
},
67+
"_my_other_ui_lib_MyOtherControl_color1": {
68+
"value": "#ffffff",
69+
"filename": "my\\other\\ui\\lib\\themes\\base\\sub-directory\\MyOtherControl.less",
70+
"rootFilename": "my\\other\\ui\\lib\\themes\\foo\\library.source.less"
71+
},
72+
"_my_other_ui_lib_MyControl_color2": {
73+
"value": "#008000",
74+
"filename": "my\\other\\ui\\lib\\themes\\foo\\MyControl.less",
75+
"rootFilename": "my\\other\\ui\\lib\\themes\\foo\\library.source.less"
76+
}
77+
};
78+
79+
const aImports = [
80+
"my\\ui\\lib\\themes\\foo\\global.less",
81+
"my\\other\\ui\\lib\\themes\\foo\\MyControl.less",
82+
"my\\other\\ui\\lib\\themes\\base\\library.source.less",
83+
"my\\ui\\lib\\themes\\base\\global.less",
84+
"my\\other\\ui\\lib\\themes\\base\\MyControl.less",
85+
"my\\other\\ui\\lib\\themes\\base\\sub-directory\\MyOtherControl.less"
86+
];
87+
88+
const variables = variableCollector.getVariables(aImports);
89+
90+
assert.deepEqual(variables, {
91+
"_my_other_ui_lib_MyControl_color1": "#ffffff",
92+
"_my_other_ui_lib_MyOtherControl_color1": "#ffffff",
93+
"_my_other_ui_lib_MyControl_color2": "#008000"
94+
}, "relevant variables should be returned");
95+
});
96+
it("should return variables when calling 'getVariables' (filename = rootFilename, no imports)", function() {
97+
const env = {};
98+
const variableCollector = new VariableCollector(env);
99+
100+
variableCollector.mVariables = {
101+
"color1": {
102+
"value": "#ffffff",
103+
"filename": "something",
104+
"rootFilename": "something"
105+
}
106+
};
107+
const aImports = [];
108+
109+
const variables = variableCollector.getVariables(aImports);
110+
111+
assert.deepEqual(variables, {
112+
"color1": "#ffffff"
113+
}, "relevant variables should be returned");
114+
});
115+
});

0 commit comments

Comments
 (0)