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

[Draft][No ticket] New linter rule to prevent leaky selectors #2351

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

Conversation

futa-ikeda
Copy link
Contributor

@futa-ikeda futa-ikeda commented Oct 10, 2024

  • Ticket: [NA]
  • Feature flag: n/a

To reviewers

  • To try this new linter rule out locally, I went through and deleted all of the scss files in the repo and just created one scss file
cd app
find . -name "*.scss" -type f -delete
cd ../lib
find . -name "*.scss" -type f -delete

Some helpful docs https://postcss.org/api/#atrule-nodes

To do

  • Finish this list of scenarios to test
  • Should raise a stink for bare element selectors (e.g. h3, li)
  • Should raise a stink for bare global selectors (e.g. :global(.class), :global(#id)
  • Should raise a stink for bare multiple element selectors (e.g. div h3, div > h3, div,\nh3)

Purpose

  • Add some custom linting to prevent any leaky css selectors

Summary of Changes

  • Add new ember-osf-web-stylelint.js to house new rule
  • Update .stylelintrc to use custom plugin

Screenshot(s)

  • When introducing a new CSS rule that may have unintended side-effects
    image

Side Effects

  • Will need to sprinkle in a lot of // stylelint-disable statements to relevant css files

QA Notes

  • No functional changes

Copy link
Contributor

@brianjgeiger brianjgeiger left a comment

Choose a reason for hiding this comment

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

Looking pretty cool so far. One thought is just in file naming. It looks like from the exports we'd only have one rule per js file. In that case, would it make more sense to have an ember-osf-web-stylelint directory and have this file be no-unlocalized-selectors.js?

@futa-ikeda
Copy link
Contributor Author

Looking pretty cool so far. One thought is just in file naming. It looks like from the exports we'd only have one rule per js file. In that case, would it make more sense to have an ember-osf-web-stylelint directory and have this file be no-unlocalized-selectors.js?

Ahh, yea, that's a good call. I named it when I was assuming I could pack as many rules into that file as I wanted, but I think you're right.

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