Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use standalone Sass task (no Gulp) #3258

Merged
merged 5 commits into from
Feb 15, 2023
Merged

Conversation

colinrotherham
Copy link
Contributor

@colinrotherham colinrotherham commented Feb 7, 2023

This PR addresses gulp-sass not supporting the Dart Sass modern compilation API

It moves our Sass compile:scss task into a separate file but also:

  1. Removes all "gulp wrapper" legacy packages
  2. Calls Node Sass and PostCSS via their JavaScript API (no Gulp)
  3. Creates a reusable compileStylesheet() helper ready for deprecation log filtering

It follows the same approach we took for Rollup:

Due to gulp-postcss we haven't been able to follow the PostCSS config options
https://github.com/postcss/postcss-load-config#options

These changes now show a clear fromto config for each stylesheet

Packages removed include:

gulp-plumber
gulp-postcss
gulp-sass
vinyl

Ticks off some tasks from:

But primarily helps unblock:

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3258 February 7, 2023 15:59 Inactive
@colinrotherham colinrotherham linked an issue Feb 7, 2023 that may be closed by this pull request
1 task
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3258 February 7, 2023 16:20 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3258 February 10, 2023 17:45 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3258 February 13, 2023 09:17 Inactive
@colinrotherham colinrotherham force-pushed the review-app-assets branch 2 times, most recently from 7e009a5 to d1efb00 Compare February 13, 2023 13:48
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3258 February 13, 2023 13:50 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3258 February 14, 2023 09:01 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3258 February 14, 2023 09:15 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3258 February 14, 2023 11:32 Inactive
Base automatically changed from review-app-assets to main February 15, 2023 12:15
@colinrotherham colinrotherham added dependencies Pull requests that update a dependency file and removed tech debt labels Feb 15, 2023
Copy link
Contributor

@domoscargin domoscargin left a comment

Choose a reason for hiding this comment

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

LGTM


// Resolve @imports via
includePaths: [
join(paths.src, 'govuk'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's take this line out, just in case it causes something unexpected to happen - can always review later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All sorted, yep happy to wait. Thank you 👍


// Resolve @imports via
includePaths: [
join(paths.src, 'govuk'),
Copy link
Contributor

Choose a reason for hiding this comment

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

See previous comment.

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3258 February 15, 2023 15:48 Inactive
@colinrotherham colinrotherham merged commit e5d00bf into main Feb 15, 2023
@colinrotherham colinrotherham deleted the standalone-sass-task branch February 15, 2023 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file sass / css tooling
Projects
Development

Successfully merging this pull request may close these issues.

3 participants