feat(sass): upgrade sass to v1.58.3

[rwaskiewicz]

Pull request checklist

Please check if your PR fulfills the following requirements:

• Docs have been reviewed and added / updated if needed (for bug fixes / features)
• Build (npm run build) was run locally and any changes were pushed
• Tests (npm test) were run locally and passed
• Prettier (npm run prettier) was run locally and passed

Pull request type

Please check the type of change your PR introduces:

• Bugfix
• Feature
• Refactoring (no functional changes, no api changes)
• Build related changes
• Documentation content changes
• Other (please describe):

What is the current behavior?

GitHub Issue Number:
Closes #227

What is the new behavior?

this commit upgrades sass to v1.58.1 from v1.42.0.

between the aforementioned versions of sass, several changes occurred in the library. some of which are accounted for in this commit, others are withheld for another day.

as of sass v1.45.0, the library produces/exports its own types, leading to the removal of the third party @types library.

this commit updates how @stencil/sass bundles sass. the shape of the output code has changed such that sass' entrypoint must call a load() function to add render (which performs the compilation step) to the prototupe. the rollup plugin that performs this action has been updated to perform this call to load(), then subsequently export render. failing to do so will result in render being undefined.

the ways in which libraries such as chokidar, readline, and immutable are used have changes as well (immutable has been added to sass since it was last upgraded). the rollup configuration to remove these from the bundle is believed to be valid, so long as we do not use the new sass javascript api (more on that coming).

sass v1.45.0 introduced a new javascript api. it is intended to the new api to supersede the old one (that this library is currently using). as a result, many of the types that we were exporting from the sass library (and its typings) have new names. often, these names are prefixed with 'Legacy', to denote that the type is deprecated. these types had little change to their constiuents, and have been called out as such when they did.

this commit does not attempt to move the project off the legacy javascript sass api at this time. this was done for a few reasons:

1. it would introduce additional churn to the commit/pull request. although this migration itself will likely be a breaking change, we attempt a large upgrade here, and would like to keep the scope minimized.
2. the stencil team must revisit the value proposition of bundling sass. the current bundling configuration is fragile, and can break at any time. this decision will affect how permissive we are in allowing different versions of sass to be used with this project.

additional jsdoc/typings for configuration (JS) files have been added to improve understanding of the rollup configuration/type saftey where possible.

Does this introduce a breaking change?

• Yes
• No

Sass often has 'potential breaking changes' in minor verisons of the library. For this upgrade, the following versions of sass may include a breaking change:

• 1.57.0
• 1.56.0
• 1.55.0
• 1.51.0
• 1.49.10
• 1.48.0

Users should be review the linked changelogs and test their libraries thoroughly. Although this upgrade may not necessarily break for consumers, it is designated as such out of caution.

Testing

In addition to unit tests, the following manual tests were performed

Stencil Styles

This branch was built & packed (npm ci && npm run build && npm pack) and installed in the Stencil Styles test. Running npm start yielded the correct page:

Ionic Framework

This branch was published under the 3.0.0-dev.2023.2.16.0 tag, and added to a draft branch for Ionic Framework (that targets v7.0.0 of Framework). All screenshot tests pass https://github.com/ionic-team/ionic-framework/pull/26815/files

Other information

Note that this branch targets the feature-3.0 branch

[tanner-reits]

lgtm

[alicewriteswrongs]

I think this all looks straightforward enough, just had one sort of question about what the require-elimination is about

[alicewriteswrongs]

sort of clarification question - in addition to the mentioned changes w/ adding immutable and the newer API, is this mainly done to reduce bundle size? If so, is it because the way Sass is written doesn't work well w/ tree-shaking?

[rwaskiewicz]

Yes, that and we don't explicitly require it (so it's not in the bundle w/o an additional rollup plugin), nor do I believe we need/use it (IIUC its only used in the new API).

I would like to get a better handle on this before/when we switch to the new Sass API (and I'd like to do it before Dart Sass 2.0). This was just the minimal needed to get Sass updated to a new version, since we hadn't touched it in so long

[alicewriteswrongs]

all makes sense I think 👍