Skip to content

Make frontend development 10-12x faster (compile SCSS through sass-embedded)#9081

Merged
microstudi merged 1 commit intodecidim:developfrom
mainio:feature/faster-scss
Apr 7, 2022
Merged

Make frontend development 10-12x faster (compile SCSS through sass-embedded)#9081
microstudi merged 1 commit intodecidim:developfrom
mainio:feature/faster-scss

Conversation

@ahukkanen
Copy link
Copy Markdown
Contributor

@ahukkanen ahukkanen commented Mar 22, 2022 •
edited
Loading

🎩 What? Why?

At #8783 the SCSS compilation speed was discussed. During a bit of comparison I figured that when bypassing Webpack during the SCSS compilation, it was 5-6x faster. Furthermore, when using the sass-embedded (instead of sass) package which uses the "embedded" version of the compiler (i.e. Dart version instead of JS version), it made the compilation even 2x faster from that.

So, in total this resulted to 10-12x faster SCSS compile times.

This got me thinking why couldn't we use sass-embedded also in Webpack. I figured it's not possible through configuring the sass-loader plugin due to some bugs, so I built our own loader to pass the SCSS files to sass-embedded instead.

The initial load time after starting webpack-dev-server drops from 1-3 minutes (mileage may vary) down to ~20s and further reloads form ~20s+ down to ~3s.

This makes developing frontend a more enjoyable experience.

Note that this does not solve any other issues with Webpacker, the related discussion is still valid. This is just a patch for the current situation that we still have to live with for some time.

Also note that Webpack is still slower than running the scss compiler directly but this is large enough improvement to the current situation.

📌 Related Issues

Testing

Run the webpack-dev-server and experience a bit of joy for the first time.

📋 Checklist

  • ❓ CONSIDER adding a unit test if your PR resolves an issue.
  • ✔️ DO check open PR's to avoid duplicates.
  • ✔️ DO keep pull requests small so they can be easily reviewed.
  • ✔️ DO build locally before pushing.
  • ✔️ DO make sure tests pass.
  • ✔️ DO make sure any new changes are documented in docs/.
  • ✔️ DO add and modify seeds if necessary.
  • ✔️ DO add CHANGELOG upgrade notes if required.
  • ✔️ DO add to GraphQL API if there are new public fields.
  • ✔️ DO add link to MetaDecidim if it's a new feature.
  • ❌AVOID breaking the continuous integration build.
  • ❌AVOID making significant changes to the overall architecture.

@andreslucena andreslucena added module: core target: developer-experience type: internal PRs that aren't necessary to add to the CHANGELOG for implementers labels Mar 23, 2022
@ahukkanen
Copy link
Copy Markdown
Contributor Author

ahukkanen commented Mar 23, 2022

@andreslucena Regarding what I told few moments ago, this is generating the import map also for production builds.

But I think this is the case with the existing configuration too, e.g. if you look at MetaDecidim:
https://meta.decidim.org/decidim-packs/css/decidim_core-d86c6f6f.css.map

So from my perspective, it's done!

andreslucena
andreslucena approved these changes Apr 4, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

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

👍🏽

A quick local check, from 30146 ms to 3972 ms 😄

@sdelcroix
Copy link
Copy Markdown
Contributor

sdelcroix commented Apr 4, 2022 •
edited
Loading

Great to hear that ! Do you think a backport to 0.26 is possible ?

I wonder how do you do to initially compile in less than a minute ? I have a standard Decidim setup, with 2 light CSS and JS files and webpacker takes 6mn on our server (a VM by the way) :/

@ahukkanen ahukkanen requested a review from microstudi April 7, 2022 07:50
microstudi
microstudi reviewed Apr 7, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@microstudi microstudi left a comment

Choose a reason for hiding this comment

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

This is a great improvement, very much needed.

When testing it, in a newly created development_app, i found that if I use the command bin/webpack-dev-server some warnings appear:
image

Compiled with problems:

WARNING

InjectManifest has been called multiple times, perhaps due to running webpack in --watch mode. The precache manifest generated after the first call may be inaccurate! Please see https://github.com/GoogleChrome/workbox/issues/1790 for more information.


WARNING

/decidim-packs/js/vendors-node_modules_decidim_decidim-bulletin_board_src_index_js.js is 2.21 MB, and won't be precached. Configure maximumFileSizeToCacheInBytes to change this limit.


WARNING

/decidim-packs/js/vendors-node_modules_graphiql_esm_index_js-node_modules_graphiql_graphiql_css.js is 2.42 MB, and won't be precached. Configure maximumFileSizeToCacheInBytes to change this limit.

@ahukkanen
Copy link
Copy Markdown
Contributor Author

ahukkanen commented Apr 7, 2022

When testing it, in a newly created development_app, i found that if I use the command bin/webpack-dev-server some warnings appear: image

@microstudi These warnings are unrelated to this change. The same warnings appear with the current configurations. They come from Webpack.

The first warning appears under the watch mode and the link provides additional information about that.

The two other warnings are what they are... There's probably some crypto libraries included in the bulletin board JS which makes it large. The GraphiQL warning could be because it compiles the CSS within the JS (at least judging by that error).

@microstudi
Copy link
Copy Markdown
Contributor

microstudi commented Apr 7, 2022

Thanks, I see that these message appear in other branches. Mergin develop over this one prevents from appearing into the browser.

@microstudi microstudi merged commit c93dae3 into decidim:develop Apr 7, 2022
@ahukkanen ahukkanen deleted the feature/faster-scss branch April 7, 2022 09:41
@ahukkanen ahukkanen mentioned this pull request Apr 7, 2022
Merged
12 tasks
entantoencuanto added a commit that referenced this pull request Apr 7, 2022
@entantoencuanto
Merge branch 'develop' into chore/meetings_optimization
630ffe2 
* develop:
  Compile SCSS through sass-embedded (#9081)
  Prevent race condition between prevenTimeout and show modal (#9092)
  Bump elections dependencies to 0.23.0 (#9140)
  Reduce d3 bundle size (#9034)
  Improve wording when casting your vote (#9098)
  Do not send upcoming meeting notification for hidden or withdrawn meetings (#9134)
  Fix processes count in processes group title cell (#9087)
  Fix attachments when called from Cells (#9136)
  Clarify message to user when checking census (#9112)
@alecslupu alecslupu added this to the 0.27.0 milestone Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@microstudi microstudi microstudi left review comments

@andreslucena andreslucena andreslucena approved these changes

Assignees

No one assigned

Labels

module: core target: developer-experience type: internal PRs that aren't necessary to add to the CHANGELOG for implementers

Projects

None yet

Milestone

0.27.0

Development

Successfully merging this pull request may close these issues.

5 participants

@ahukkanen @sdelcroix @microstudi @andreslucena @alecslupu