alphagov / govuk-prototype-kit

Rapidly create HTML prototypes of GOV.UK services
https://prototype-kit.service.gov.uk
MIT License
306 stars 236 forks source link

Sass errors are not obvious as the kit uses previously compiled sass in .tmp #2170

Closed joelanman closed 1 year ago

joelanman commented 1 year ago

Description of the issue

I think this is a regression - we had this previously and fixed it: #295

If you have any error in your sass, the kit will continue to run using css compiled in .tmp. It will however fail to deploy as that involves rendering from scratch.

Steps to reproduce the issue

  1. Run the kit without sass errors
  2. Introduce a sass error

Actual vs expected behaviour

Actual: The kit appears to run normally - new sass changes will not appear but that is not always obvious Expected: The error should be clear to the user - the kit will fail to deploy in this state

Environment (where applicable)

nataliecarey commented 1 year ago

What I think this ticket asks for is:

The current behaviour is:

When cloning a kit with broken scss:

/Users/natalie.carey/projects/prototype-kits/2023-05-23/node_modules/govuk-prototype-kit/lib/utils/index.js:189
      throw new Error(`File ${filename} does not exist`)
            ^

Error: File /Users/natalie.carey/projects/prototype-kits/2023-05-23/.tmp/public/stylesheets/application.css does not exist
    at waitUntilFileExists (/Users/natalie.carey/projects/prototype-kits/2023-05-23/node_modules/govuk-prototype-kit/lib/utils/index.js:189:13)
    at async Object.runDevServer (/Users/natalie.carey/projects/prototype-kits/2023-05-23/node_modules/govuk-prototype-kit/lib/dev-server.js:36:3)
    at async runDev (/Users/natalie.carey/projects/prototype-kits/2023-05-23/node_modules/govuk-prototype-kit/bin/cli:399:3)

In this case the kit doesn't start and there's no in-browser error page

When there has previously been a successful sass build but now it's broken:

Pages are rendered with the old compiled css.

In these cases I believe we should be showing an error page rather than a page that looks correct but is not reflecting the current state of the project.

@joelanman do you agree with what I've put here? I think it's the same thing you're expecting but I just want to make sure.

joelanman commented 1 year ago

@nataliecarey yep that sounds correct. I think what we might need to consider since the last time it was fixed is the raw error is not very clear:

throw new Error(`File ${filename} does not exist`)

it should be clear that the error is in the sass

joelanman commented 1 year ago

came up in support today