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 @blueprintjs/stylelint-plugin package and no-prefix-literal rule #4683

Merged
merged 13 commits into from
Apr 29, 2021

Conversation

p-szm
Copy link
Contributor

@p-szm p-szm commented Apr 28, 2021

Checklist

  • Includes tests
  • Update documentation

Changes proposed in this pull request:

This PR adds a stylelint rule that bans the usage of .bp3/.bp4 prefixes in favor of using the bp-ns variables. The rule comes with an autofixer that automatically converts the literal prefixes to the bp-ns variable and also inserts the Blueprint variables import.

Screenshot

bp-stylelint

Copy link
Contributor

@adidahiya adidahiya left a comment

Choose a reason for hiding this comment

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

overall looks good! After seeing this rule fixer work in practice, I think we should introduce a more verbose variable which consumers import... $ns feels a little to terse to be littering all over BP consumer code via the autofixer. it could be a simple alias in core/src/common/_variables.scss:

$ns: bp3 !default;
// alias for BP users outside this repo
$bp-ns: $ns;

@@ -0,0 +1,18 @@
/* Copyright 2020 Palantir Technologies, Inc. All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: formatting of block comment is slightly off (is the autofixer broken?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fixed this manually, but actually it seems like this is also a valid format and the autofixer always does it this way on my computer. Could you test what happens on your machine?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like the plugin only inserts starts at the beginning and end https://github.com/Stuk/eslint-plugin-header/blob/66f5269c8a7e3282afd02b914b502f7bb95fe702/lib/rules/header.js#L46

Do you have a template for ts/js files configured in VS code by any chance?

packages/stylelint-plugin/src/index.ts Outdated Show resolved Hide resolved
packages/stylelint-plugin/test/checkImportExists.test.js Outdated Show resolved Hide resolved
packages/stylelint-plugin/test/no-prefix-literal.test.js Outdated Show resolved Hide resolved
@styu
Copy link
Contributor

styu commented Apr 28, 2021

@adidahiya i love the idea of introducing $bp-ns or something like that instead of requiring $ns, since the latter is fairly generic in retrospect

@adidahiya adidahiya merged commit 27772ed into palantir:develop Apr 29, 2021
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.

3 participants