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.

Sign up for GitHub

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

USWDS-Compile - Sass: Create quiet deprecations setting #131

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

USWDS-Compile - Sass: Create quiet deprecations setting #131

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
35b48cb
Create quietDeps setting
mahoneycm Nov 25, 2024
6d0329b
Update sass-embedded to test new deprecation warnings
mahoneycm Nov 25, 2024
ae7ffa0
Set default setting to true
mahoneycm Nov 25, 2024
23901df
Add quietSassDeps setting to settings table
mahoneycm Dec 9, 2024
1074780
Update README.md
mahoneycm Dec 10, 2024
7a4c434
Rename setting to sassDeprecationWarnings and update logic to be more…
mahoneycm Dec 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion gulpfile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ let settings = {
},
browserslist: ["> 2%", "last 2 versions", "IE 11", "not dead"],
sassSourcemaps: true,
quietSassDeps: true
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Wondering if we can improve clarity with the setting name here to highlight that this is about Sass "warnings", not just "deps" (This also clarifies that we are talking about deprecations, not dependencies).

Also wondering if it should be written in the negative, so that we can set "false" as a default. This part is very much a quibble and could just be personal preference, but it felt a bit odd to add a settings.compile.quietSassDeps = false to the gulpfile.

Curious what you think!

Suggested change
quietSassDeps: true
sassDeprecationWarnings: false

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm I'm divided here. While I do think that the naming convention your suggesting makes sense, part of me thinks we should match what Sass is providing. Since quietDeps is the name of setting I see potential value in keeping ours close to it.

Do you see any value in changing it to sassQuietDeps so we're sort of "prefixing" the Sass setting name with "Sass"?

Alternatively, we use this approach which would probably be more readable for users unfamiliar with this Sass deprecation 🤔

sassDeprecationWarnings: false
...
sass({
  ...
  quietDeps: !settings.compile.sassDeprecationWarnings,
})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed in 7a4c434!

},
sprite: {
width: 24,
Expand Down Expand Up @@ -198,7 +199,7 @@ function buildSass() {
sass({
outputStyle: "compressed",
includePaths: buildSettings.includes,
silenceDeprecations: ["mixed-decls"]
quietDeps: settings.compile.quietSassDeps,
}).on("error", handleError)
)
.pipe(replace(/\buswds @version\b/g, `based on uswds v${pkg}`))
Expand Down
199 changes: 111 additions & 88 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading