google / google/site-kit-wp

Resolve Sass deprecation warnings in first-party SCSS and prevent regressions

Open
#13,352 1 comment 0 reactions 0 assignees View on GitHub
P1 Type: Infrastructure
Dominant language
JavaScript
Stars
1.4k
Forks
383
Avg merge
4d 14h
Merged PRs (30d)
77

Description

## Feature Description

`npm run build:dev` currently emits **81 Sass deprecation warnings plus 1,251 suppressed repeats** (Dart Sass 1.99). The volume makes the build output effectively unreadable and hides genuine warnings.

The warnings come from two sources, and only one is actionable:

| Deprecation | `@material` (vendor) | First-party (`assets/`) | Removed in |
| --- | --- | --- | --- |
| `@import` | 0 | 20 | Dart Sass 3.0 |
| Global built-ins (`map-get`, `percentage`) | 9 | 10 | Dart Sass 3.0 |
| `/` division | 10 | 5 | **Dart Sass 2.0** |
| `if()` syntax | 15 | 0 | Dart Sass 3.0 |
| `red()`/`green()`/`blue()` | 9 | 0 | Dart Sass 3.0 |

The vendor half cannot be fixed here: `@material` is pinned at **1.1.0** (MDC Web v1.x, 2019). Upstream resolved these in MDC 14.x, which is a rewrite, not an upgrade we can take incidentally. `@material` is our only `node_modules` SCSS dependency, so the vendor noise is a single known-stale package.

This issue covers the half we control: silencing the unactionable vendor noise, fixing the first-party deprecations, and making them fail the build if reintroduced.

**Out of scope** (follow-up issues): migrating `@import` → `@use`/`@forward`, and upgrading `@material`. The `@import` migration is blocked on the MDC upgrade — our partials rely on MDC globals (`$mdc-layout-grid-columns`, `$mdc-layout-grid-default-margin`) being ambiently available, and MDC 1.x is not module-system compatible.

### This is almost entirely automated

`sass-migrator` does the source changes. The two commands in the Implementation Brief handle **every** first-party occurrence reachable from the build entrypoints, and all four compiled stylesheets are **identical** before and after. Budget review time, not discovery time.

### Why silencing vendor warnings is safe

Verified against Dart Sass 1.99:

* `quietDeps: true` suppresses **deprecation warnings** in dependencies only — first-party files still warn normally.
* It does **not** suppress errors. A dependency with a genuine fault (undefined mixin) still throws with `quietDeps: true`. When a removed feature eventually breaks `@material`, the build fails loudly rather than silently emitting wrong CSS.
* `sass` is pinned to `^1.30.0` in `assets/package.json` and `storybook/package.json`, so `npm install` cannot pull 2.0/3.0. That upgrade is a deliberate act.

Known trade-off: `quietDeps: true` **overrides** `fatalDeprecations` for dependencies, so deps are exempt from build-level enforcement. This is accepted — first-party code is the nearer blocker for a Dart Sass 2.0 upgrade regardless.

---------------

_Do not alter or remove anything below. The following sections will be managed by moderators only._

## Acceptance criteria

* `npm run build:dev` completes with **zero Sass deprecation warnings** other than the `@import` warnings that are explicitly out of scope.
* No deprecation warnings originating in `node_modules/@material` appear in build output.
* Deprecation warnings originating in first-party `assets/**/*.scss` are **not** silenced — the suppression is scoped to dependencies only.
* Newly introduced first-party `/` division or global built-in usage **fails the build** rather than warning.
* The compiled CSS in `dist/assets/css/` is unchanged before and after the change.
* The unused `_googlesitekit-modules-list.scss` partial is removed.
* No visual change should be present after the edit.

## Implementation Brief

- [ ] Run the division migrator across all four SCSS entrypoints (`admin.scss`, `adminbar.scss`, `wpdashboard.scss`, `authorize-application.scss`), passing `node_modules` as the load path so the `@material` imports resolve:

npx sass-migrator division --migrate-deps -I node_modules assets/sass/{admin,adminbar,wpdashboard,authorize-application}.scss

* Add `--dry-run` first to preview the file list.
* The migrator chooses per case: multiplication by an exact reciprocal (`$x / 2` → `$x * 0.5`, `/ 4` → `* 0.25`, `/ 100` → `* 0.01`) where that is exactly equivalent, and `math.div()` where it is not (e.g. `/ 12`). **Leave its choice alone** — do not hand-normalise everything to `math.div()`. It adds `@use "sass:math";` only to the files that need it.
* Despite `--migrate-deps`, this does **not** modify anything under `node_modules/`. Spot-check the diff to confirm.

- [ ] Run the built-in migrator across the same entrypoints. `--built-in-only` converts global functions **without** performing the out-of-scope `@import` → `@use` migration:

npx sass-migrator module --built-in-only --migrate-deps -I node_modules assets/sass/{admin,adminbar,wpdashboard,authorize-application}.scss

* Handles `map-get` → `map.get`, `map-keys` → `map.keys`, `percentage` → `math.percentage`, adding the relevant `@use` lines.
* Leaves `rgba()` alone, correctly — it is a CSS function and remains global in Sass.

- [ ] Spot-check for anything the migrator could not reach. It walks the `@import` graph from the entrypoints, so a partial that nothing imports is invisible to it, while grep still finds it:

grep -rn --include="*.scss" -E "[\$a-z0-9\)] / [\$0-9]" assets/
grep -rn --include="*.scss" -E "(map-get|map-keys|percentage)\(" assets/

* Ignore matches inside `calc()`, `rgb()`/`rgba()` colour syntax, `aspect-ratio`, and comments — those are valid CSS, not Sass division.
* This surfaces exactly one file the migrator skips, `components/global/_googlesitekit-modules-list.scss`, which is dead code — see the next step. Any *other* file it surfaces is a genuine gap and must be fixed by hand.

- [ ] Delete `assets/sass/components/global/_googlesitekit-modules-list.scss` rather than migrating it.
* Nothing imports the partial, and `googlesitekit-modules-list` appears nowhere else in the repository — not in SCSS, JS, or PHP.
* Every rule in the file is nested inside `.googlesitekit-modules-list`, so the selectors it defines for otherwise-live classes (`.googlesitekit-settings-connect-module__*`, `.googlesitekit-settings-module__*`) only apply as descendants of a class that is never rendered. Those classes keep their own styles from other partials.
* Because the file is not imported, it contributes nothing to the compiled output, so removing it does not affect the "compiled CSS is unchanged" criterion.

- [ ] Note that `assets/sass/vendor/` holds first-party copies of vendor code. These are **not** covered by `quietDeps` and are migrated like any other first-party file.

- [ ] In `assets/webpack/adminCss.config.js`, `assets/webpack/gutenbergBlocks.config.js`, and `storybook/main.js`, add to the existing `sass-loader` `sassOptions`:
* `quietDeps: true` — suppresses deprecation warnings from files resolved through `includePaths` (i.e. `@material`). Verified not to affect first-party files.
* `fatalDeprecations: [ 'slash-div', 'global-builtin' ]` — escalates these two to hard errors. Verified to still fire for first-party code while `quietDeps` is on.
* Apply this step **last**. Adding it before the migrations above are complete will fail the build.

### Test Coverage

* No new automated tests required — this is a build-configuration and mechanical-refactor change with no behavioural surface.
* Regression protection comes from `fatalDeprecations`, which turns these two deprecations from "warns forever" into "build fails" — stronger than a test could provide.
* Verify that enforcement works by temporarily reintroducing a `$var / 2` in any first-party partial and confirming `npm run build:dev` fails.
* The compiled CSS is the correctness check: the migrator's rewrites are arithmetically equivalent, so the files in `dist/assets/css/` must not change. Build on `develop`, copy them aside, build on the feature branch, and diff.

## QA Brief

*

## Changelog entry

*

Contributor guide

Open the contributing guide

Research direction

Start with the four SCSS entrypoints under assets/sass and the sass-loader configurations in assets/webpack/adminCss.config.js, assets/webpack/gutenbergBlocks.config.js, and storybook/main.js. Preview and run the two listed sass-migrator commands, inspect grep results, and remove assets/sass/components/global/_googlesitekit-modules-list.scss. Run npm run build:dev and compare dist/assets/css/ against develop; done means only out-of-scope @import warnings remain and first-party deprecations fail the build.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, sass, webpack
Domain
build-system, tooling
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
70/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.