Replace deprecated node-sass with sass #161813

patrykkopycinski · 2023-07-12T21:12:36Z

Summary

https://sass-lang.com/blog/libsass-is-deprecated/

Most of the changes related to
https://sass-lang.com/documentation/breaking-changes/slash-div/

delanni · 2023-07-13T12:56:17Z

Also, removing this would save us from having to build and serve node-sass binaries for the supported platforms. cc: @elastic/kibana-operations

.buildkite/pipelines/on_merge.yml

jbudz

Thanks for looking into this! Have we considered using sass-embedded instead of sass?

It looks like there's almost a 3x slowdown in performance between node-sass and sass

mistic

I agree with @jbudz here. We should experiment to use sass-embedded instead of sass. It offers the same upsides for us ( 1- move ahead from a legacy thing ; 2 - include prebuilds for every important architecture) while it should preserve the previous performance on builds

patrykkopycinski · 2023-07-13T14:33:14Z

@jbudz @mistic @delanni unfortunately we are not able to use sass-embedded, because sass-loader in current version doesn't support sass-embedded and we cannot bump the loader, becuase from ^11.0.0 it requires webpack@5 🤷‍♂️

mistic · 2023-07-15T02:17:09Z

I'm just worried this is adding a performance penalty to a such important job step like Build Kibana Distribution and Plugins. In some cases this is adding 7 minutes to that job (from 13 min to 20) which will impact have impact on other jobs like FTR (and possible the rest of the ci). @jbudz what are your thoughts here? I'm divided on move on and deal with the consequences until we upgrade to webpack 5 (and can use the embedded) or hold this off until we upgrade webpack and then use this PR with the sass embedded instead of this one.

jbudz · 2023-07-17T15:41:38Z

I'm just worried this is adding a performance penalty to a such important job step like Build Kibana Distribution and Plugins. In some cases this is adding 7 minutes to that job (from 13 min to 20) which will impact have impact on other jobs like FTR (and possible the rest of the ci). @jbudz what are your thoughts here? I'm divided on move on and deal with the consequences until we upgrade to webpack 5 (and can use the embedded) or hold this off until we upgrade webpack and then use this PR with the sass embedded instead of this one.

node-sass is still under maintenance. If there's a must-have feature I think we can make the case for upgrading, but in isolation I don't see a benefit over the significant time increase in plugin builds. Overall it looks like +4 minutes on CI, and a matching (and less parallel) experience for development.

It is good to know we can upgrade now though, if needed. The previous attempt didn't work out #61722.

mistic · 2023-07-18T00:16:30Z

I agree with Jon here. I think this is good work but the benefits of doing it now without upgrading the webpack to v5 so we can use the embedded version don't pay the costs (CI overhead) IMHO. It's probably better to hold off this PR until that moment and then we can move on with it

…ode-sass # Conflicts: # yarn.lock

patrykkopycinski · 2023-08-07T21:38:49Z

woooooah, great news @mistic!

mistic · 2023-12-07T00:13:08Z

@patrykkopycinski can we revive this? v10.5.0 was released and I think we can now use it instead of the node_module patch solution https://www.npmjs.com/package/sass-loader/v/10.5.0

…rkdown # Conflicts: # yarn.lock

mistic

lgtm

…ode-sass # Conflicts: # package.json # yarn.lock

…-fix'

kibana-ci · 2023-12-19T13:47:25Z

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`canvas`	1013.7KB	1013.7KB	-2.0B
`cloudSecurityPosture`	451.7KB	451.7KB	+12.0B
`controls`	200.2KB	200.2KB	-6.0B
`dashboard`	376.4KB	376.4KB	+4.0B
`dataViewManagement`	120.0KB	120.0KB	-2.0B
`dataVisualizer`	617.6KB	617.6KB	+10.0B
`discover`	592.2KB	592.2KB	+14.0B
`enterpriseSearch`	2.7MB	2.7MB	-52.0B
`eventAnnotationListing`	196.8KB	196.8KB	+16.0B
`filesManagement`	89.9KB	89.9KB	-4.0B
`graph`	387.3KB	387.3KB	+8.0B
`home`	178.6KB	178.6KB	-14.0B
`indexManagement`	585.1KB	585.2KB	+6.0B
`infra`	1.9MB	1.9MB	-4.0B
`ingestPipelines`	360.8KB	360.8KB	-4.0B
`kibanaOverview`	46.6KB	46.6KB	-4.0B
`kibanaReact`	214.5KB	214.7KB	+194.0B
`lens`	1.4MB	1.4MB	+4.0B
`logExplorer`	981.7KB	981.7KB	+10.0B
`management`	75.0KB	75.0KB	-4.0B
`maps`	2.9MB	2.9MB	+36.0B
`metricsDataAccess`	85.2KB	85.2KB	-4.0B
`ml`	3.6MB	3.6MB	+28.0B
`monitoring`	462.2KB	462.3KB	+66.0B
`observabilityShared`	36.3KB	36.3KB	-4.0B
`osquery`	1.0MB	1.0MB	-4.0B
`presentationUtil`	82.1KB	82.1KB	+10.0B
`security`	575.2KB	575.2KB	+16.0B
`securitySolution`	11.3MB	11.3MB	+2.0B
`securitySolutionEss`	42.7KB	42.7KB	-4.0B
`securitySolutionServerless`	361.1KB	361.1KB	-4.0B
`spaces`	174.9KB	174.9KB	-4.0B
`stackAlerts`	80.6KB	80.6KB	+10.0B
`unifiedDocViewer`	58.4KB	58.4KB	+6.0B
`unifiedSearch`	213.7KB	213.7KB	+8.0B
`visDefaultEditor`	141.0KB	140.9KB	-14.0B
`visTypeTimelion`	38.8KB	38.8KB	-4.0B
`visTypeVega`	1.8MB	1.8MB	-4.0B
`visTypeVislib`	339.7KB	339.7KB	+40.0B
`visualizations`	266.9KB	266.9KB	-28.0B
total			+330.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`core`	375.5KB	375.6KB	+76.0B
`embeddable`	78.2KB	78.2KB	+20.0B
`esUiShared`	155.8KB	155.8KB	-4.0B
`globalSearchBar`	26.3KB	26.3KB	-6.0B
`kbnUiSharedDeps-npmDll`	6.2MB	6.2MB	+280.0B
`kibanaOverview`	15.2KB	15.1KB	-24.0B
`kibanaReact`	41.8KB	41.8KB	-6.0B
`osquery`	51.2KB	51.2KB	+10.0B
`searchprofiler`	18.7KB	18.8KB	+18.0B
total			+364.0B

History

💔 Build #184219 failed ab38475
💔 Build #183791 failed 0f89f75
💚 Build #183392 succeeded 3fc60a6
💚 Build #182540 succeeded bd47e8b
💔 Build #182534 failed 4f1c6d1
💔 Build #182445 failed 17532ad

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @patrykkopycinski

This reverts commit 87d5d6b.

jbudz · 2023-12-21T16:01:07Z

@patrykkopycinski This was reverted with 3def20e. See https://buildkite.com/elastic/kibana-artifacts-snapshot/builds/3735 for build failures, and https://buildkite.com/elastic/kibana-pull-request/builds/184823#018c8d10-8206-4937-9122-e13d5616f81a for the followup failures after the output style was changed.

patrykkopycinski · 2023-12-23T00:33:02Z

Sorry @jbudz, fixed version #173942
Maybe we could add "Build Kibana artifacts" pipeline to the PR check to avoid that in the future?

## Summary The previous PR #161813 was reverted due to the broken webpack config eef1afc --------- Co-authored-by: Tiago Costa <tiago.costa@elastic.co> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Jon <jon@elastic.co>

## Summary The previous PR elastic#161813 was reverted due to the broken webpack config elastic@eef1afc --------- Co-authored-by: Tiago Costa <tiago.costa@elastic.co> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Jon <jon@elastic.co> (cherry picked from commit d4be2a3)

The previous PR elastic#161813 was reverted due to the broken webpack config elastic@eef1afc --------- Co-authored-by: Tiago Costa <tiago.costa@elastic.co> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Jon <jon@elastic.co>

patrykkopycinski added 3 commits July 12, 2023 23:11

Replace deprecated node-sass with sass

8d42da3

fix

4757eff

fix

eb127d3

patrykkopycinski marked this pull request as ready for review July 13, 2023 10:37

patrykkopycinski requested review from a team as code owners July 13, 2023 10:37

formgeist approved these changes Jul 13, 2023

View reviewed changes

decrease timeout

69d3dfb

patrykkopycinski self-assigned this Jul 13, 2023

patrykkopycinski added the release_note:skip Skip the PR/issue when compiling release notes label Jul 13, 2023

delanni reviewed Jul 13, 2023

View reviewed changes

.buildkite/pipelines/on_merge.yml Outdated Show resolved Hide resolved

jbudz reviewed Jul 13, 2023

View reviewed changes

mistic reviewed Jul 13, 2023

View reviewed changes

revert

9724bd9

patrykkopycinski force-pushed the chore/replace-node-sass branch from ad0c75f to 9724bd9 Compare July 13, 2023 14:30

patrykkopycinski and others added 4 commits July 13, 2023 19:06

timeout

3179c11

fix

e6bf726

timeout

0bd8025

Merge branch 'main' into chore/replace-node-sass

c918a67

patrykkopycinski added 4 commits July 20, 2023 20:18

sass-embedded

d2b44de

Merge branch 'main' of github.com:elastic/kibana into chore/replace-n…

c061e79

…ode-sass # Conflicts: # yarn.lock

Update casing_check_config.js

fcb95da

Update renovate.json

acd566a

watson mentioned this pull request Nov 23, 2023

[Ops] Bump node-sass from v8.0.0 to v9.0.0 #171863

Closed

patrykkopycinski added 4 commits December 7, 2023 12:12

Merge branch 'main' of github.com:elastic/kibana into feat/mermaid-ma…

bce1d9a

…rkdown # Conflicts: # yarn.lock

test

33e3f7c

fix

17532ad

fix

4f1c6d1

ThomThomson approved these changes Dec 7, 2023

View reviewed changes

fix

bd47e8b

mistic approved these changes Dec 8, 2023

View reviewed changes

patrykkopycinski and others added 4 commits December 13, 2023 12:43

Merge branch 'main' into chore/replace-node-sass

3fc60a6

Merge branch 'main' into chore/replace-node-sass

0f89f75

Merge branch 'main' of github.com:elastic/kibana into chore/replace-n…

ab38475

…ode-sass # Conflicts: # package.json # yarn.lock

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

df12625

…-fix'

MichaelMarcialis approved these changes Dec 19, 2023

View reviewed changes

patrykkopycinski merged commit 87d5d6b into elastic:main Dec 21, 2023
37 checks passed

patrykkopycinski deleted the chore/replace-node-sass branch December 21, 2023 11:52

kibanamachine added v8.13.0 backport:skip This commit does not require backporting labels Dec 21, 2023

jbudz mentioned this pull request Dec 21, 2023

Fix canvas runtime sass outputStyle #173850

Closed

jbudz added a commit that referenced this pull request Dec 21, 2023

Revert "Replace deprecated node-sass with sass (#161813)"

3def20e

This reverts commit 87d5d6b.

jbudz added the reverted label Dec 21, 2023

patrykkopycinski restored the chore/replace-node-sass branch December 23, 2023 00:26

patrykkopycinski mentioned this pull request Dec 23, 2023

Replace deprecated node-sass with sass #2 #173942

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace deprecated node-sass with sass #161813

Replace deprecated node-sass with sass #161813

patrykkopycinski commented Jul 12, 2023 •

edited

Loading

delanni commented Jul 13, 2023

jbudz left a comment

mistic left a comment

patrykkopycinski commented Jul 13, 2023

mistic commented Jul 15, 2023

jbudz commented Jul 17, 2023 •

edited

Loading

mistic commented Jul 18, 2023

patrykkopycinski commented Aug 7, 2023

mistic commented Dec 7, 2023

mistic left a comment

kibana-ci commented Dec 19, 2023

jbudz commented Dec 21, 2023

patrykkopycinski commented Dec 23, 2023

Replace deprecated node-sass with sass #161813

Replace deprecated node-sass with sass #161813

Conversation

patrykkopycinski commented Jul 12, 2023 • edited Loading

Summary

delanni commented Jul 13, 2023

jbudz left a comment

Choose a reason for hiding this comment

mistic left a comment

Choose a reason for hiding this comment

patrykkopycinski commented Jul 13, 2023

mistic commented Jul 15, 2023

jbudz commented Jul 17, 2023 • edited Loading

mistic commented Jul 18, 2023

patrykkopycinski commented Aug 7, 2023

mistic commented Dec 7, 2023

mistic left a comment

Choose a reason for hiding this comment

kibana-ci commented Dec 19, 2023

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

History

jbudz commented Dec 21, 2023

patrykkopycinski commented Dec 23, 2023

patrykkopycinski commented Jul 12, 2023 •

edited

Loading

jbudz commented Jul 17, 2023 •

edited

Loading