sass-loader icon indicating copy to clipboard operation
sass-loader copied to clipboard

Make it easier to prefer `sass-embedded` over `sass`

Open G-Rath opened this issue 1 year ago • 2 comments

[!NOTE]

This could be a "when sass-embedded support is no longer experimental" modification

Modification Proposal

My understanding is that sass-embedded can be considered like-for-like to sass in terms of features and output i.e. assuming you can run sass-embedded, you can always use it instead of sass and get the exact same compiled output (just faster); and the main reason that sass is a thing is because sass-embedded has more complicated OS requirements being a compiled binary.

Thankfully package.json has a way of expressing dependencies which are desirable but OS restricted and so whose incompatibility with the current OS that is being installed on should not be considered a failure, allowing us to specify sass-embedded as a dependency alongside sass as a fallback i.e.

{
  "name": "my-app",
  "devDependencies": {
    "sass": "^1.69.5",
    "sass-loader": "^13.3.2",
    "webpack": "^5.0.0",
    "webpack-cli": "^4.0.0"
  },
  "optionalDependencies": {
    "sass-embedded": "^1.70.0"
  }
}

However it seems that sass-loader always favors loading sass, so in the above it'll never use sass-embedded by default:

https://github.com/webpack-contrib/sass-loader/blob/bd65cbadfc6330e9752525a420a286d7c18fbd57/src/utils.js#L4-L25

Now this is technically documented (though in the context of node-sass) and we can explicitly change this using implementation; however that would require us to have resolving logic in our webpack config across all applications and projects.

I figure sass was preferred back when it was just node-sass because the latter is technically deprecated and is not like-for-like, but maybe it's time to consider revising the default implementation order with the introduction of sass-embedded?

I think there are a few ways this could be done which would all be fine:

  • just change the default order to look for sass-embedded first
  • introduce a new implementation on the lines of prefer-sass-embedded (or a dedicated preferSassEmbedded boolean option)
  • support a comma list of implementations to try first to allow specifying order: sass-embedded,sass (though this is probably a bit much given there's not expected to be yet another sass implementation...)

Expected Behavior / Situation

sass-loader prefers sass-embedded by default, allowing us to manage providing the fastest sass implementation possible for the host, such as with optionalDependencies or installing sass-embedded in a dedicated step during building our docker images, provisioning servers, etc.

Actual Behavior / Situation

sass-loader loads sass by default, ignoring sass-embedded

Please paste the results of npx webpack-cli info here, and mention other relevant information


  System:
    OS: Linux 5.15 Ubuntu 22.04.3 LTS 22.04.3 LTS (Jammy Jellyfish)
    CPU: (20) x64 13th Gen Intel(R) Core(TM) i7-13700H
    Memory: 12.61 GB / 15.46 GB
  Binaries:
    Node: 18.16.0 - ~/.nodenv/versions/18.16.0/bin/node
    Yarn: 1.22.19 - ~/.nodenv/versions/18.16.0/bin/yarn
    npm: 9.5.1 - ~/.nodenv/versions/18.16.0/bin/npm
  Packages:
    sass-loader: ^13.3.2 => 13.3.2 
    webpack: ^5.0.0 => 5.89.0 
    webpack-cli: ^4.0.0 => 4.10.0 

G-Rath avatar Jan 20 '24 22:01 G-Rath

sass-embedded is still not very stable and a new API (which we need to use in future) is still under constuction, that is why we prefer sass right now

But I am fine with

support a comma list of implementations to try first to allow specifying order: sass-embedded,sass (though this is probably a bit much given there's not expected to be yet another sass implementation...)

alexander-akait avatar Jan 22 '24 12:01 alexander-akait

So feel free to send a PR

alexander-akait avatar Jan 22 '24 12:01 alexander-akait

For what it's worth, the Sass team considers sass-embedded to be stable and production-ready.

nex3 avatar Jul 19 '24 01:07 nex3

@nex3 I was about to ask for receipts but then checked your profile so I guess this is the receipt 😅 (not intended rudely, just being able to point to a blog post or mainteinr comment saying "it's official!" in my experience makes things very clear)

@alexander-akait given this signal from the lead designer/developer, would you be happy for me to do a pr swapping the import check to look for embedded sass first? Or are there still concerns you'd like to see addressed?

G-Rath avatar Jul 20 '24 02:07 G-Rath