Skip to content

Conversation

@frankieroberto
Copy link
Contributor

@frankieroberto frankieroberto commented Jun 3, 2025

The tests currently just run a code linter and validate the prototype plugin config but don’t actually compile the sass at all.

This adds a compile of the sass to the tests.

There might be a better way to do this, but using the sass command line tool seemed simplest?

cc @colinrotherham @paulrobertlloyd

"lint:fix": "npm run lint:prettier:fix && npm run lint:js:fix && npm run lint:scss:fix",
"test": "npm run lint && npx govuk-prototype-kit@latest validate-plugin",
"test": "npm run lint && npx govuk-prototype-kit@latest validate-plugin && npm run test:css",
"test:css": "npx sass --pkg-importer=node --load-path=. --quiet-deps src/x-govuk/index.scss ./tmp/output.css",

Choose a reason for hiding this comment

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

Worth flagging that --quiet-deps uses loads paths to determine dependencies

It's a downside of --load-path=. and silences local warnings 😭

You might want to consider:

  1. Adding Sass CLI --fatal-deprecation
    E.g. See Sass config from NHS.UK frontend

  2. Checking output for uncompiled Sass variables (or functions)
    E.g. See Sass GitHub Action from NHS.UK frontend

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@colinrotherham is it better to drop --quiet-deps from the test compile then?

Choose a reason for hiding this comment

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

Yeah, unfortunately

At least until the day everyone uses pkg: or drops node_modues from load paths

- @import "node_modules/govuk-frontend/dist/govuk";
+ @import "govuk-frontend/dist/govuk";

@paulrobertlloyd paulrobertlloyd force-pushed the main branch 4 times, most recently from 3dbed5b to bfb92b5 Compare June 15, 2025 15:51
This should help us catch where the sass doesn't compile for some reason?
@paulrobertlloyd paulrobertlloyd force-pushed the add-sass-build-to-tests branch from 6661068 to e9cb936 Compare June 15, 2025 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants