Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace node-sass with sass-embedded #5338

Merged
merged 1 commit into from
Nov 14, 2023
Merged

Replace node-sass with sass-embedded #5338

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions .github/workflows/build_and_test_workflow.yml
View file
Open in desktop
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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can workflow files take comments? An explanation here might be helpful for if/when GitHub improves their Windows infra and we can remove these.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am pretty sure they can. i will add something.

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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a followup task to update this to a "released" version of your patch, rather than an rc tag?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only reason I used rc was to discourage the use of it by others as i noticed some of my other patches were being used.

"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
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion packages/osd-optimizer/src/integration_tests/__snapshots__/basic_optimization.test.ts.snap
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion packages/osd-optimizer/src/worker/webpack.config.ts
View file
Open in desktop
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
View file
Open in desktop
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) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ugh, I can't wait to get rid of this whole package...

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
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion packages/osd-ui-framework/src/components/local_nav/_local_search.scss
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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}`);
AMoo-Miki marked this conversation as resolved.
Show resolved Hide resolved
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)',
AMoo-Miki marked this conversation as resolved.
Show resolved Hide resolved
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
View file
Open in desktop
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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the changes above, do we have to add calc before any of the sass calculation?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just for division operations - see https://sass-lang.com/documentation/breaking-changes/slash-div/


.euiIcon {
vertical-align: baseline;
Expand Down
2 changes: 1 addition & 1 deletion src/core/public/styles/_ace_overrides.scss
View file
Open in desktop
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
8 changes: 4 additions & 4 deletions src/plugins/data/public/ui/filter_bar/_global_filter_item.scss
View file
Open in desktop
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
6 changes: 3 additions & 3 deletions src/plugins/data/public/ui/saved_query_management/_saved_query_management_component.scss
View file
Open in desktop
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
}
2 changes: 1 addition & 1 deletion src/plugins/opensearch_dashboards_react/public/markdown/_markdown.scss
View file
Open in desktop
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
View file
Open in desktop
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
6 changes: 3 additions & 3 deletions src/plugins/vis_builder/public/application/components/data_tab/dropbox.scss
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion src/plugins/vis_builder/public/application/components/workspace.scss
View file
Open in desktop
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
View file
Open in desktop
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
Loading