Visitar URL original
USWDS - Core: Remove commented code from utility builder by aduth · Pull Request #5209 · uswds/uswds · GitHub
Skip to content

USWDS - Core: Remove commented code from utility builder - #5209

Merged
thisisdano merged 1 commit into
uswds:developfrom
aduth:aduth-rm-commented-utility-builder-code
May 8, 2023
Merged

thisisdano merged 1 commit into
uswds:developfrom
aduth:aduth-rm-commented-utility-builder-code

Conversation

@aduth

@aduth aduth commented Mar 29, 2023 •

Copy link
Copy Markdown
Contributor

Summary

Reduce size of compiled CSS when comments aren't explicitly excluded.

Problem statement

When compiling USWDS without optimization configured to remove comments, the output includes 1500 instances of the following snippet of code (one for each utility class):

/*
@if map-deep-get($utility, settings, responsive) {
  @include render-media-queries(
    $utility,
    $selector,
    $property,
    $value,
    $val-props
  );
}
*/

This unnecessarily inflates the size of the compiled output, and may cause extra work for the compilation itself.

Solution

Remove unused code.

Testing and review

mkdir tmp-uswds
cd tmp-uswds
npm init -y
npm i @uswds/uswds
brew install sass/sass/sass
echo "@forward 'uswds'; @use 'uswds-core' as *;" > styles.scss
sass --load-path=node_modules/@uswds/uswds/packages styles.scss | wc -c

Before: 1336223 bytes (1336.2kb)
After: 657293 bytes (657.3kb)
Diff: 678930 bytes (-50.8%)

@mejiaj mejiaj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Makes sense to remove them if we're not using. Thanks!

@mejiaj mejiaj added Added during Sprint Historical record of work added during a sprint. Not required for current triage. Package: Utilities Work affecting Utilities. Apply only when this area helps route or find the issue. Context: Sass Issue is in Sass labels Apr 7, 2023
@mejiaj mejiaj self-assigned this Apr 7, 2023
@amyleadem amyleadem added this to the uswds 3.5.0 milestone Apr 7, 2023

@amyleadem amyleadem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This feels safe to remove. The render-media-queries mixin no longer exists in the code base, and this code block was commented out 3 years ago.

@mahoneycm mahoneycm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I saw a major reduction in the file size of the project 👍

@thisisdano thisisdano left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Little change, big effect

@thisisdano
thisisdano merged commit e3c7004 into uswds:develop May 8, 2023
@amyleadem amyleadem mentioned this pull request Jun 7, 2023
@aduth
aduth deleted the aduth-rm-commented-utility-builder-code branch August 29, 2023 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Added during Sprint Historical record of work added during a sprint. Not required for current triage. Context: Sass Issue is in Sass Package: Utilities Work affecting Utilities. Apply only when this area helps route or find the issue.

Projects

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants