Skip to content

Update build-sass dependencies#9366

Merged
aduth merged 2 commits intomainfrom
aduth-update-build-sass-deps
Oct 16, 2023
Merged

Update build-sass dependencies#9366
aduth merged 2 commits intomainfrom
aduth-update-build-sass-deps

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Oct 12, 2023

🛠 Summary of changes

Updates dependencies of @18f/identity-build-sass to latest versions.

Why?

  • Hopefully addressing some crashes that have been observed in sass-embedded in local development and in deployed instances (see Slack discussion)
  • To prepare for a release of pending changes to the package

📜 Testing Plan

Verify stylesheets build successfully:

  1. yarn build:css

Verify no visual regressions in the application

  1. make run
  2. Visit http://localhost:3000

aduth added 2 commits October 12, 2023 08:28
changelog: Internal, Dependencies, Update dependencies to latest versions
import { Readable } from 'node:stream';
import { pipeline } from 'node:stream/promises';
import sass from 'sass-embedded';
import { compile as sassCompile } from 'sass-embedded';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change was to address a deprecation which was causing some output from the build command:

`import sass from 'sass'` is deprecated.
Please use `import * as sass from 'sass'` instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

The rename from compile to sassCompile is just for readability, not a collision, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The rename from compile to sassCompile is just for readability, not a collision, right?

Correct, since we use a couple different processing tools (Sass, LightningCSS), I wanted to avoid potential ambiguity of what "compile" refers to, and follow the convention I'd introduced on the following line of with lightningTransform ➡️ toolOperation ➡️ sassCompile.

Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM

import { Readable } from 'node:stream';
import { pipeline } from 'node:stream/promises';
import sass from 'sass-embedded';
import { compile as sassCompile } from 'sass-embedded';
Copy link
Contributor

Choose a reason for hiding this comment

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

The rename from compile to sassCompile is just for readability, not a collision, right?

@aduth aduth merged commit 625c2ec into main Oct 16, 2023
@aduth aduth deleted the aduth-update-build-sass-deps branch October 16, 2023 13:23
@aduth
Copy link
Contributor Author

aduth commented Oct 16, 2023

Published to NPM as @18f/identity-build-sass@2.0.0

https://www.npmjs.com/package/@18f/identity-build-sass?activeTab=versions

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.

2 participants