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

Add "no global scoped selector" rule for CSS Modules #414

Merged
merged 5 commits into from
May 8, 2024

Conversation

iansan5653
Copy link
Contributor

@iansan5653 iansan5653 commented May 8, 2024

Adds a new Stylelint rule for CSS modules: css-modules/no-global-scoped-selector. This ensures that all selectors in a CSS module file are locally scoped - that is, they fall underneath a class selector that will be hashed.

The point of CSS modules is to encapsulate styles, so global selectors directly violate that principle. By enforcing encapsulation we can ensure that components minimize their side effects on the rest of the page. I think it's a good idea to start enforcing this early to teach best practices in the beginning.

From the docs:

✅ Example of local-scoped selectors:

// SCSS

.container {
  // ...
}

.container {
  :global .external-class {
    // ...
  }
}

❌ Example of global-scoped selectors:

// SCSS

h1 {
  // ...
}

:global .external-class {
  // ...
}

h1 {
  :global .external-class {
    // ...
  }
}

Note

This plugin requires "stylelint": "^13.0.0 || ^14.0.0" as a peer dependency. We are on 16.3.1, so I had to install the plugin with an override. I did try to fix this upstream but the package is not maintained anymore so I was unable to. Everything seems to work fine in 16.x though.

@iansan5653 iansan5653 requested a review from a team as a code owner May 8, 2024 14:12
@iansan5653 iansan5653 requested a review from langermank May 8, 2024 14:12
Copy link

changeset-bot bot commented May 8, 2024

🦋 Changeset detected

Latest commit: 4d1f3d1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/stylelint-config Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@iansan5653
Copy link
Contributor Author

iansan5653 commented May 8, 2024

Hmmm....npm i --force allowed it to be installed locally but didn't configure the package-lock.json to allow installing with npm ci without force. Is there a way around this?

Update: Resolved using overrides field in package.json

Copy link
Member

@jonrohan jonrohan left a comment

Choose a reason for hiding this comment

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

Awesome thanks for the contribution

@jonrohan jonrohan merged commit 308d8e7 into main May 8, 2024
4 checks passed
@jonrohan jonrohan deleted the no-global-scoped-selector branch May 8, 2024 16:29
@primer-css primer-css mentioned this pull request May 8, 2024
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.

None yet

2 participants