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

feat: switch from scss to postcss #5888

Merged
merged 33 commits into from
Sep 24, 2023
Merged

feat: switch from scss to postcss #5888

merged 33 commits into from
Sep 24, 2023

Conversation

ovflowd
Copy link
Member

@ovflowd ovflowd commented Sep 23, 2023

Description

To simplify the stack of the Node.js Website, I've decided to remove support for SASS/SCSS from this repository.

This is because all our styles were processed at least 3 times. (Sass -> PostCSS -> CSS -> Minification)

Introducing Tailwind would also create inconsistencies, as we use custom Tailwind "syntax" such as @tailwind, @apply and theme.

The SASS/SCSS processor also includes a lot of extra things that we don't necessarily need;

By switching to PostCSS, we wanted to retain a SCSS-alike syntax, such as supporting Mixins, Variables, Nesting, and more. This PR also adds relevant plugins although it is important to mention that the only significant difference is how mixins work with PostCSS.

The benefits of PostCSS include a lighter CSS footprint, faster compilation times, less verbose, and the support of a subset of functionalities (plugins) we like.

Differences in Mixins

In SCSS, you define a Mixin with @mixin, and if you have variables, you define them as $variable; With PostCSS, you define a Mixin with @define-mixin, and variables do not need the $.

In SCSS, you consume a Mixin with @include. In PostCSS, you consume a Mixin with @mixin.

What changed in this PR?

  • Full removal of SCSS/Sass support
    • Removed packages, renamed files from *.scss to .css
  • Added PostCSS plugins with features that benefit our day-to-day development
  • Updated any codebase that required to be patched for the functionality to work
  • Removed a few unused SCSS files
  • Updated Storybook preview.tsx, fixing styles, types, and other things
  • Updated Storybook Dark/Light Mode support for the Preview Iframe
  • Updated Storybook to use a default viewport
  • Updated Docs and all references to SCSS to be PostCSS
  • Updated relevant packages and package.json file
  • Updated Stylelint Configuration
  • Prettied/Linted all CSS files
  • Added shareable minimal-compatible VSCode Configuration
  • Updated Collaborator Guidelines with PostCSS
  • Updated TypeScript configuration for Storybook

Why so many changes?

Some changes are partially unrelated to the migration from SCSS to PostCSS (mainly the Storybook changes); they're added in this PR primarily for convenience and to address these stylistic changes in a single batch.

Validation

Storybooks, Builds, and Local Development should work as expected.

@vercel
Copy link

vercel bot commented Sep 23, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nodejs-org ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 24, 2023 7:37pm

@ovflowd ovflowd added design website redesign Issue/PR part of the Node.js Website Redesign github_actions:pull-request Trigger Pull Request Checks labels Sep 23, 2023
@github-actions github-actions bot removed github_actions:pull-request Trigger Pull Request Checks labels Sep 23, 2023
@github-actions
Copy link

github-actions bot commented Sep 23, 2023

Unit Test Coverage Report

Lines Statements Branches Functions
Coverage: 92%
90.24% (185/205) 75% (30/40) 86.66% (39/45)

Unit Test Report

Tests Skipped Failures Errors Time
14 0 💤 0 ❌ 0 🔥 3.453s ⏱️

Copy link
Contributor

@AugustinMauroy AugustinMauroy left a comment

Choose a reason for hiding this comment

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

Sounds good to me 👍

COLLABORATOR_GUIDE.md Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@nodejs nodejs deleted a comment from github-actions bot Sep 23, 2023
@ovflowd ovflowd added the github_actions:pull-request Trigger Pull Request Checks label Sep 23, 2023
Copy link
Collaborator

@bmuenzenmeyer bmuenzenmeyer left a comment

Choose a reason for hiding this comment

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

some small nits worth changing

.vscode/extensions.json Outdated Show resolved Hide resolved
.vscode/settings.json Outdated Show resolved Hide resolved
COLLABORATOR_GUIDE.md Outdated Show resolved Hide resolved
COLLABORATOR_GUIDE.md Outdated Show resolved Hide resolved
COLLABORATOR_GUIDE.md Outdated Show resolved Hide resolved
COLLABORATOR_GUIDE.md Outdated Show resolved Hide resolved
Signed-off-by: Brian Muenzenmeyer <[email protected]>
package.json Show resolved Hide resolved
Copy link
Member

@MattIPv4 MattIPv4 left a comment

Choose a reason for hiding this comment

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

Seems like a logical change to me, I'm always a fan of less tools in the chain

@ovflowd
Copy link
Member Author

ovflowd commented Sep 24, 2023

@bmuenzenmeyer and @MattIPv4 if you two don't mind, could y'all give a last copy-check on the changes? I've just pushed some changes to simplify and better some docs

@bmuenzenmeyer
Copy link
Collaborator

if you two don't mind, could y'all give a last copy-check on the changes?

✅ looking good to me

@ovflowd ovflowd added the github_actions:pull-request Trigger Pull Request Checks label Sep 24, 2023
@github-actions github-actions bot removed the github_actions:pull-request Trigger Pull Request Checks label Sep 24, 2023
@ovflowd ovflowd added this pull request to the merge queue Sep 24, 2023
Merged via the queue into main with commit f903606 Sep 24, 2023
13 of 14 checks passed
@ovflowd ovflowd deleted the feat/post-css-remove-sass branch September 24, 2023 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design infrastructure Issues/PRs related to the Repository Infra website redesign Issue/PR part of the Node.js Website Redesign
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants