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

[Polaris Lighthouse] Add migration to insert disable comments for @shopify/stylelint-polaris #7726

Merged
merged 1 commit into from
Dec 1, 2022

Conversation

sophschneider
Copy link
Contributor

@sophschneider sophschneider commented Nov 15, 2022

NOTE: even though this PR is independent of the stylelint-polaris v5 release, I recommend we hold off on shipping this until v5 to avoid confusion with the migration, because running this script with v4 and v5 yield different results

WHY are these changes introduced?

Fixes #7504

When we turn the stylelint-polaris linter on in codebases, we need a way to ignore current warnings and errors since most if not all of these failures would require some sort of refactor (there isn't a one-to-one mapping of fixes).

This will ensure that refactors do not block us from turning on the linter in order to "stop the bleeding" and prevent further regressions.

WHAT is this pull request doing?

Creates a new styles migration called styles-insert-stylelint-disable that goes through the given sass files and inserts a disable comment like:

+ // stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
padding: 1rem;

There is also logic to deal with edge cases like if there is already an existing stylelint-dsable-next-line comment, it will merge the descriptions. See more about the edge cases in the tests, or in the polaris-react draft pr below (I left comments with some examples)

How to 🎩

  1. if testing in polaris-react, turn off the overrides in .stylelintrc.js
  2. run npx stylelint 'polaris-react/**/*.scss' and note all the failures
  3. run yarn build && node ./bin/migrator-cli.js styles-insert-stylelint-disable 'polaris-react/**/*.scss'
  4. run npx stylelint 'polaris-react/**/*.scss' and note the reduced failures

If using a snapshot, run the migrator with npx @shopify/polaris-migrator styles-insert-stylelint-disable <path>. The migrator just uses a simple @shopify/polaris-stylelint config, so if double checking with npx stylelint make sure you are running the same version with the same config

@sophschneider sophschneider force-pushed the sophie/styles-insert-stylelint-disable branch 7 times, most recently from 6fba7ad to 0902b22 Compare November 17, 2022 22:05
@Shopify Shopify deleted a comment from github-actions bot Nov 17, 2022
@Shopify Shopify deleted a comment from github-actions bot Nov 17, 2022
@sophschneider sophschneider force-pushed the sophie/styles-insert-stylelint-disable branch 4 times, most recently from 6ecc142 to 4ff1162 Compare November 18, 2022 21:02
@Shopify Shopify deleted a comment from github-actions bot Nov 18, 2022
@sophschneider sophschneider force-pushed the sophie/styles-insert-stylelint-disable branch from 4ff1162 to f5ba66f Compare November 18, 2022 21:36
@sophschneider
Copy link
Contributor Author

/snapit

@github-actions
Copy link
Contributor

github-actions bot commented Nov 18, 2022

🫰✨ Thanks @sophschneider! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/plugin-polaris@0.0.0-snapshot-release-20221118220013
yarn add @shopify/polaris-migrator@0.0.0-snapshot-release-20221118220013

Copy link
Member

@sam-b-rose sam-b-rose left a comment

Choose a reason for hiding this comment

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

This is awesome! Great job testing it out in web and polaris-react 💯

@sophschneider sophschneider force-pushed the sophie/styles-insert-stylelint-disable branch 5 times, most recently from ae65ddc to 4230529 Compare December 1, 2022 22:41
@sophschneider sophschneider force-pushed the sophie/styles-insert-stylelint-disable branch from 4230529 to f37db53 Compare December 1, 2022 23:00
@sophschneider sophschneider merged commit 22c4107 into main Dec 1, 2022
@sophschneider sophschneider deleted the sophie/styles-insert-stylelint-disable branch December 1, 2022 23:16
mrcthms pushed a commit that referenced this pull request Dec 6, 2022
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @shopify/stylelint-polaris@5.0.0

### Major Changes

- [#7691](#7691)
[`38b2a00a6`](38b2a00)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Stylelint
Polaris v5 release

## @shopify/polaris-icons@6.7.0

### Minor Changes

- [#7816](#7816)
[`afe77e584`](afe77e5)
Thanks [@allyshiasewdat](https://github.com/allyshiasewdat)! - Add
returns major icon

## @shopify/polaris-migrator@0.10.0

### Minor Changes

- [#7726](#7726)
[`22c4107b3`](22c4107)
Thanks [@qt314](https://github.com/qt314)! - Added migration to insert
disable comments for @shopify/stylelint-polaris

### Patch Changes

- [#7836](#7836)
[`77736370e`](7773637)
Thanks [@qt314](https://github.com/qt314)! - Decouple polaris migrator
test from stylelint config so it's easier to maintain

- Updated dependencies
\[[`38b2a00a6`](38b2a00)]:
    -   @shopify/stylelint-polaris@5.0.0

## @shopify/polaris@10.14.0

### Minor Changes

- [#7833](#7833)
[`e47595193`](e475951)
Thanks [@mrcthms](https://github.com/mrcthms)! - Separated BulkActions
and SelectAllActions for new sticky bulk actions experience


- [#7812](#7812)
[`e51d2c2d2`](e51d2c2)
Thanks [@chazdean](https://github.com/chazdean)! - Created `Divider`
component with guidance and examples

### Patch Changes

- [#7763](#7763)
[`28529baaf`](28529ba)
Thanks [@n-filatov](https://github.com/n-filatov)! - Update the Toast
component to a more compact layout

- Updated dependencies
\[[`afe77e584`](afe77e5)]:
    -   @shopify/polaris-icons@6.7.0

## @shopify/plugin-polaris@0.0.19

### Patch Changes

- Updated dependencies
\[[`22c4107b3`](22c4107),
[`77736370e`](7773637)]:
    -   @shopify/polaris-migrator@0.10.0

## polaris.shopify.com@0.27.0

### Minor Changes

- [#7812](#7812)
[`e51d2c2d2`](e51d2c2)
Thanks [@chazdean](https://github.com/chazdean)! - Created `Divider`
component with guidance and examples

### Patch Changes

- Updated dependencies
\[[`afe77e584`](afe77e5),
[`e47595193`](e475951),
[`28529baaf`](28529ba),
[`e51d2c2d2`](e51d2c2)]:
    -   @shopify/polaris-icons@6.7.0
    -   @shopify/polaris@10.14.0

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
juzser pushed a commit to juzser/polaris that referenced this pull request Jul 27, 2023
…opify/stylelint-polaris (Shopify#7726)

**NOTE: even though this PR is independent of the [stylelint-polaris
v5](https://github.com/Shopify/polaris/pull/7691/files) release, I
recommend we hold off on shipping this until v5 to avoid confusion with
the migration, because running this script with v4 and v5 yield
different results**

### WHY are these changes introduced?

Fixes Shopify#7504

When we turn the stylelint-polaris linter on in codebases, we need a way
to ignore current warnings and errors since most if not all of these
failures would require some sort of refactor (there isn't a one-to-one
mapping of fixes).

This will ensure that refactors do not block us from turning on the
linter in order to "stop the bleeding" and prevent further regressions.

### WHAT is this pull request doing?

Creates a new styles migration called `styles-insert-stylelint-disable`
that goes through the given sass files and inserts a disable comment
like:

```diff
+ // stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
padding: 1rem;
```

There is also logic to deal with edge cases like if there is already an
existing `stylelint-dsable-next-line` comment, it will merge the
descriptions. See more about the edge cases in the tests, or in the
`polaris-react` draft pr below (I left comments with some examples)

### How to 🎩
* Test on `polaris-react` Shopify#7773
(3140 failures -> 4 failures)
* Test on `shopify/web` https://github.com/Shopify/web/pull/78162 (30383
failures -> 9 failures)

1. if testing in polaris-react, turn off the `overrides` in
`.stylelintrc.js`
2. run `npx stylelint 'polaris-react/**/*.scss'` and note all the
failures
3. run `yarn build && node ./bin/migrator-cli.js
styles-insert-stylelint-disable 'polaris-react/**/*.scss'`
4. run `npx stylelint 'polaris-react/**/*.scss'` and note the reduced
failures

If using a snapshot, run the migrator with `npx
@shopify/polaris-migrator styles-insert-stylelint-disable <path>`. The
migrator just uses a simple `@shopify/polaris-stylelint` config, so if
double checking with `npx stylelint` make sure you are running the same
version with the same config
juzser pushed a commit to juzser/polaris that referenced this pull request Jul 27, 2023
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @shopify/stylelint-polaris@5.0.0

### Major Changes

- [Shopify#7691](Shopify#7691)
[`38b2a00a6`](Shopify@38b2a00)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Stylelint
Polaris v5 release

## @shopify/polaris-icons@6.7.0

### Minor Changes

- [Shopify#7816](Shopify#7816)
[`afe77e584`](Shopify@afe77e5)
Thanks [@allyshiasewdat](https://github.com/allyshiasewdat)! - Add
returns major icon

## @shopify/polaris-migrator@0.10.0

### Minor Changes

- [Shopify#7726](Shopify#7726)
[`22c4107b3`](Shopify@22c4107)
Thanks [@qt314](https://github.com/qt314)! - Added migration to insert
disable comments for @shopify/stylelint-polaris

### Patch Changes

- [Shopify#7836](Shopify#7836)
[`77736370e`](Shopify@7773637)
Thanks [@qt314](https://github.com/qt314)! - Decouple polaris migrator
test from stylelint config so it's easier to maintain

- Updated dependencies
\[[`38b2a00a6`](Shopify@38b2a00)]:
    -   @shopify/stylelint-polaris@5.0.0

## @shopify/polaris@10.14.0

### Minor Changes

- [Shopify#7833](Shopify#7833)
[`e47595193`](Shopify@e475951)
Thanks [@mrcthms](https://github.com/mrcthms)! - Separated BulkActions
and SelectAllActions for new sticky bulk actions experience


- [Shopify#7812](Shopify#7812)
[`e51d2c2d2`](Shopify@e51d2c2)
Thanks [@chazdean](https://github.com/chazdean)! - Created `Divider`
component with guidance and examples

### Patch Changes

- [Shopify#7763](Shopify#7763)
[`28529baaf`](Shopify@28529ba)
Thanks [@n-filatov](https://github.com/n-filatov)! - Update the Toast
component to a more compact layout

- Updated dependencies
\[[`afe77e584`](Shopify@afe77e5)]:
    -   @shopify/polaris-icons@6.7.0

## @shopify/plugin-polaris@0.0.19

### Patch Changes

- Updated dependencies
\[[`22c4107b3`](Shopify@22c4107),
[`77736370e`](Shopify@7773637)]:
    -   @shopify/polaris-migrator@0.10.0

## polaris.shopify.com@0.27.0

### Minor Changes

- [Shopify#7812](Shopify#7812)
[`e51d2c2d2`](Shopify@e51d2c2)
Thanks [@chazdean](https://github.com/chazdean)! - Created `Divider`
component with guidance and examples

### Patch Changes

- Updated dependencies
\[[`afe77e584`](Shopify@afe77e5),
[`e47595193`](Shopify@e475951),
[`28529baaf`](Shopify@28529ba),
[`e51d2c2d2`](Shopify@e51d2c2)]:
    -   @shopify/polaris-icons@6.7.0
    -   @shopify/polaris@10.14.0

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
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.

Create a rollout strategy
3 participants