Skip to content

Commit

Permalink
Replace node-sass with sass-embedded (opensearch-project#5338)
Browse files Browse the repository at this point in the history
* Dart's `sass` uses a lot more memory that `node-sass` which causes failures with CI on Windows when building platform plugins: pagefile size was bumped.

Signed-off-by: Miki <miki@amazon.com>
  • Loading branch information
AMoo-Miki authored Nov 14, 2023
1 parent 524fd93 commit f822702
Show file tree
Hide file tree
Showing 28 changed files with 257 additions and 563 deletions.
32 changes: 32 additions & 0 deletions .github/workflows/build_and_test_workflow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,14 @@ jobs:
run: |
git config --global core.autocrlf false
- name: Configure pagefile size (Windows only)
if: matrix.os == 'windows-latest'
uses: al-cheb/configure-pagefile-action@v1.3
with:
minimum-size: 16GB
maximum-size: 64GB
disk-root: "C:"

- name: Checkout code
uses: actions/checkout@v3

Expand Down Expand Up @@ -146,6 +154,14 @@ jobs:
run: |
git config --global core.autocrlf false
- name: Configure pagefile size (Windows only)
if: matrix.os == 'windows-latest'
uses: al-cheb/configure-pagefile-action@v1.3
with:
minimum-size: 16GB
maximum-size: 64GB
disk-root: "C:"

- name: Checkout code
uses: actions/checkout@v3

Expand Down Expand Up @@ -232,6 +248,14 @@ jobs:
run: |
git config --global core.autocrlf false
- name: Configure pagefile size (Windows only)
if: matrix.os == 'windows-latest'
uses: al-cheb/configure-pagefile-action@v1.3
with:
minimum-size: 16GB
maximum-size: 64GB
disk-root: "C:"

- name: Checkout code
uses: actions/checkout@v3

Expand Down Expand Up @@ -334,6 +358,14 @@ jobs:
git config --global core.autocrlf false
working-directory: .

- name: Configure pagefile size (Windows only)
if: matrix.os == 'windows-latest'
uses: al-cheb/configure-pagefile-action@v1.3
with:
minimum-size: 16GB
maximum-size: 64GB
disk-root: "C:"

- name: Checkout code
uses: actions/checkout@v3
with:
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Update webpack environment targets ([#4649](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4649))
- Add @curq as maintainer ([#4760](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4760))
- Bump `oui` to `1.3.0` ([#4941](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4941))
- Replace `node-sass` with `sass-embedded` ([#5338](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5338))
- Add @bandinib-amzn as maintainer ([#5113](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5113))
- Add @bandinib-amzn to CODEOWNERS file. ([#5456](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5456))
- Bump `chromedriver` from `107.0.3` to `119.0.1` ([#5465](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5465))
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@
"dependencies": {
"@aws-crypto/client-node": "^3.1.1",
"@elastic/datemath": "5.0.3",
"@elastic/eui": "npm:@opensearch-project/oui@1.3.0",
"@elastic/eui": "npm:@opensearch-project/oui@1.4.0-alpha.2",
"@elastic/good": "^9.0.1-kibana3",
"@elastic/numeral": "^2.5.0",
"@elastic/request-crypto": "2.0.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/osd-interpreter/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"del": "^6.1.1",
"getopts": "^2.2.5",
"pegjs": "0.10.0",
"sass-loader": "npm:@amoo-miki/sass-loader@10.4.1-node-sass-9.0.0-libsass-3.6.5",
"sass-loader": "npm:@amoo-miki/sass-loader@10.4.1-node-sass-9.0.0-libsass-3.6.5-with-sass-embedded.rc1",
"style-loader": "^1.1.3",
"supports-color": "^7.0.0",
"url-loader": "^2.2.0",
Expand Down
4 changes: 2 additions & 2 deletions packages/osd-optimizer/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@
"css-loader": "^5.2.7",
"file-loader": "^6.2.0",
"loader-utils": "^2.0.4",
"node-sass": "npm:@amoo-miki/node-sass@9.0.0-libsass-3.6.5",
"sass-embedded": "1.66.1",
"postcss-loader": "^4.2.0",
"raw-loader": "^4.0.2",
"sass-loader": "npm:@amoo-miki/sass-loader@10.4.1-node-sass-9.0.0-libsass-3.6.5",
"sass-loader": "npm:@amoo-miki/sass-loader@10.4.1-node-sass-9.0.0-libsass-3.6.5-with-sass-embedded.rc1",
"style-loader": "^1.1.3",
"url-loader": "^2.2.0",
"val-loader": "^2.1.2",
Expand Down

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion packages/osd-optimizer/src/worker/webpack.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ export function getWebpackConfig(bundle: Bundle, bundleRefs: BundleRefs, worker:
)};\n${content}`;
},
webpackImporter: false,
implementation: require('node-sass'),
implementation: require('sass-embedded'),
sassOptions: {
outputStyle: 'compressed',
includePaths: [Path.resolve(worker.repoRoot, 'node_modules')],
Expand Down
52 changes: 21 additions & 31 deletions packages/osd-ui-framework/Gruntfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
*/

const { strip } = require('comment-stripper');
const sass = require('node-sass');
const sass = require('sass-embedded');
const postcss = require('postcss');
const postcssConfig = require('@osd/optimizer/postcss.config.js');

Expand Down Expand Up @@ -70,6 +70,26 @@ module.exports = function (grunt) {

grunt.registerTask('prodBuild', ['clean:target', 'copy:makeProdBuild', 'babel:prodBuild']);

const uiFrameworkCompile = async (src, dest) => {
try {
const { css: compiledCSS } = await sass.compileAsync(src);
const result = await postcss([postcssConfig]).process(
strip(compiledCSS, { language: 'css' }),
{
from: src,
to: dest,
}
);

grunt.file.write(dest, result.css);
if (result.map) {
grunt.file.write(`${dest}.map`, result.map);
}
} catch (ex) {
grunt.log.error(ex);
}
};

grunt.registerTask('compileCss', function () {
const done = this.async();
Promise.all([
Expand All @@ -79,34 +99,4 @@ module.exports = function (grunt) {
uiFrameworkCompile('src/kui_next_dark.scss', 'dist/kui_next_dark.css'),
]).then(done);
});

function uiFrameworkCompile(src, dest) {
return new Promise((resolve) => {
sass.render(
{
file: src,
},
function (error, result) {
if (error) {
grunt.log.error(error);
}

postcss([postcssConfig])
.process(strip(result.css.toString('utf8'), { language: 'css' }), {
from: src,
to: dest,
})
.then((result) => {
grunt.file.write(dest, result.css);

if (result.map) {
grunt.file.write(`${dest}.map`, result.map);
}

resolve();
});
}
);
});
}
};
6 changes: 3 additions & 3 deletions packages/osd-ui-framework/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,16 @@
"enzyme-adapter-react-16": "^1.9.1"
},
"devDependencies": {
"@elastic/eui": "npm:@opensearch-project/oui@1.3.0",
"@elastic/eui": "npm:@opensearch-project/oui@1.4.0-alpha.2",
"@osd/babel-preset": "1.0.0",
"@osd/optimizer": "1.0.0",
"comment-stripper": "^0.0.4",
"grunt": "^1.5.2",
"grunt-babel": "^8.0.0",
"grunt-contrib-clean": "^2.0.0",
"grunt-contrib-copy": "^1.0.0",
"node-sass": "npm:@amoo-miki/node-sass@9.0.0-libsass-3.6.5",
"sass-loader": "npm:@amoo-miki/sass-loader@10.4.1-node-sass-9.0.0-libsass-3.6.5",
"sass-embedded": "1.66.1",
"sass-loader": "npm:@amoo-miki/sass-loader@10.4.1-node-sass-9.0.0-libsass-3.6.5-with-sass-embedded.rc1",
"postcss": "^8.4.5",
"sinon": "^7.4.2"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
*/
.kuiLocalSearchAssistedInput__assistance {
position: absolute;
right: $kuiFormControlHorizontalPadding / 2;
right: calc($kuiFormControlHorizontalPadding / 2);
top: 50%; /* 1 */
z-index: 2;
transform: translateY(-50%); /* 1 */
Expand Down
8 changes: 4 additions & 4 deletions packages/osd-ui-shared-deps/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
},
"dependencies": {
"@elastic/charts": "31.1.0",
"@elastic/eui": "npm:@opensearch-project/oui@1.3.0",
"@elastic/eui": "npm:@opensearch-project/oui@1.4.0-alpha.2",
"@elastic/numeral": "^2.5.0",
"@opensearch/datemath": "5.0.3",
"@osd/i18n": "1.0.0",
Expand Down Expand Up @@ -46,9 +46,9 @@
"css-loader": "^5.2.7",
"del": "^6.1.1",
"loader-utils": "^2.0.4",
"node-sass": "npm:@amoo-miki/node-sass@9.0.0-libsass-3.6.5",
"sass-loader": "npm:@amoo-miki/sass-loader@10.4.1-node-sass-9.0.0-libsass-3.6.5",
"sass-embedded": "1.66.1",
"sass-loader": "npm:@amoo-miki/sass-loader@10.4.1-node-sass-9.0.0-libsass-3.6.5-with-sass-embedded.rc1",
"val-loader": "^2.1.2",
"webpack": "npm:@amoo-miki/webpack@4.46.0-rc.2"
}
}
}
40 changes: 40 additions & 0 deletions scripts/postinstall.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,48 @@ const removeUnwantedFolders = async (root, unwantedNames) => {

return promises;
};

const patchFile = async (file, patch) => {
console.log(`Patching ${file}`);
const patches = Array.isArray(patch) ? patch : [patch];
let fileContent = await fs.readFile(file, 'utf8');
for (const { from, to } of patches) {
// The splitting by `to` is to make sure we don't patch already patched ones
fileContent = fileContent
.split(to)
.map((token) => token.split(from))
.flat()
.join(to);
}
await fs.writeFile(file, fileContent);
};

const run = async () => {
const promises = await removeUnwantedFolders('node_modules', ['demo', 'example', 'examples']);

promises.push(
patchFile('node_modules/font-awesome/scss/_variables.scss', {
from: '(30em / 14)',
to: 'calc(30em / 14)',
})
);
promises.push(
patchFile('node_modules/@elastic/charts/dist/theme.scss', [
{
from: '$legendItemVerticalPadding / 2',
to: 'calc($legendItemVerticalPadding / 2)',
},
{
from: '$echLegendRowGap / 2',
to: 'calc($echLegendRowGap / 2)',
},
{
from: '$euiBorderRadius / 2',
to: 'calc($euiBorderRadius / 2)',
},
])
);

await Promise.all(promises);
};

Expand Down
2 changes: 1 addition & 1 deletion src/core/public/core_app/styles/_mixins.scss
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
text-align: center;
background-color: $euiColorEmptyShade;
border-radius: 100%;
padding: $euiSize / 2;
padding: calc($euiSize / 2);

.euiIcon {
vertical-align: baseline;
Expand Down
2 changes: 1 addition & 1 deletion src/core/public/styles/_ace_overrides.scss
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
// it is being scoped by a known outer selector
.coreSystemRootDomElement {
.ace-tm {
$aceBackground: tintOrShade($euiColorLightShade, 50%, 0);
$aceBackground: tintOrShade($euiColorLightShade, 50%, 0%);

background-color: $euiColorLightestShade;
color: $euiTextColor;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
line-height: $euiSize;
border: none;
color: $euiTextColor;
padding-top: $euiSizeM / 2;
padding-bottom: $euiSizeM / 2;
padding-top: calc($euiSizeM / 2);
padding-bottom: calc($euiSizeM / 2);
white-space: normal; /* 1 */

.euiBadge__childButton {
Expand Down Expand Up @@ -67,8 +67,8 @@
left: 0;
width: $euiSizeXS;
background-color: $osdGlobalFilterItemBorderColor;
border-top-left-radius: $euiBorderRadius / 2;
border-bottom-left-radius: $euiBorderRadius / 2;
border-top-left-radius: calc($euiBorderRadius / 2);
border-bottom-left-radius: calc($euiBorderRadius / 2);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@

.osdSavedQueryManagement__pagination {
justify-content: center;
padding: ($euiSizeM / 2) $euiSizeM $euiSizeM;
padding: calc($euiSizeM / 2) $euiSizeM $euiSizeM;
}

.osdSavedQueryManagement__text {
padding: $euiSizeM $euiSizeM ($euiSizeM / 2) $euiSizeM;
padding: $euiSizeM $euiSizeM calc($euiSizeM / 2) $euiSizeM;
}

.osdSavedQueryManagement__list {
Expand All @@ -23,5 +23,5 @@
max-height: inherit; // Fixes overflow for applied max-height
// Left/Right padding is calculated to match the left alignment of the
// popover text and buttons
padding: ($euiSizeM / 2) $euiSizeXS !important; // Override flush
padding: calc($euiSizeM / 2) $euiSizeXS !important; // Override flush
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
$osdDefaultFontSize: 14px;

@function canvasToEm($size) {
@return #{$size / $osdDefaultFontSize}em;
@return #{calc($size / $osdDefaultFontSize)}em;
}

.osdMarkdown__body {
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/vis_builder/public/application/app.scss
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
}

&__resizeButton {
transform: translateX(-$euiSizeM / 2);
transform: translateX(calc(-1 * $euiSizeM / 2));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@

&__container {
display: grid;
grid-gap: $euiSizeXS / 2;
padding: $euiSizeS - ($euiSizeXS / 2) $euiSizeS $euiSizeS $euiSizeS;
grid-gap: calc($euiSizeXS / 2);
padding: calc($euiSizeS - ($euiSizeXS / 2)) $euiSizeS $euiSizeS $euiSizeS;
background-color: $euiColorLightShade;
border-radius: $euiBorderRadius;
}
Expand All @@ -32,7 +32,7 @@
}

&__draggable {
padding: $euiSizeXS / 2 0;
padding: calc($euiSizeXS / 2) 0;
animation: pop-in $euiAnimSpeedSlow $euiAnimSlightResistance forwards;
transform-origin: bottom;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
$animation-time: 3;
$animation-multiplier: 5;
$total-duartion: $animation-time * $animation-multiplier;
$keyframe-multiplier: 1 / $animation-multiplier;
$keyframe-multiplier: calc(1 / $animation-multiplier);

.vbWorkspace {
display: grid;
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/vis_default_editor/public/_agg_params.scss
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
.visEditorAggParam--half {
margin: $euiSize 0;
display: inline-block;
width: calc(50% - #{$euiSizeS / 2});
width: calc(50% - #{calc($euiSizeS / 2)});
}

.visEditorAggParam--half-size {
Expand Down
Loading

0 comments on commit f822702

Please sign in to comment.