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

Fix cache refresh when config is changed #6356

Merged
merged 15 commits into from Sep 27, 2022

Conversation

kimulaco
Copy link
Member

Which issue, if any, is this issue related to?

Closes #2908

Is there anything in the PR that needs further explanation?

Currently, there is a problem with cached file messages not being displayed. This report already exists. #4715
So the test I have added is not optimal. I don't know how to prove that the file is cached in the CLI.

@changeset-bot
Copy link

changeset-bot bot commented Sep 20, 2022

🦋 Changeset detected

Latest commit: 4b5b1c0

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

This PR includes changesets to release 1 package
Name Type
stylelint Patch

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

@kimulaco
Copy link
Member Author

kimulaco commented Sep 21, 2022

This lint error does not occur local. I'm checking this error.
https://github.com/stylelint/stylelint/actions/runs/3094063323/jobs/5007077131

@kimulaco
Copy link
Member Author

kimulaco commented Sep 21, 2022

The reason the error did not occur on my local was that this change was not merged.
I fixed type error, please review.

Copy link
Member

@ybiquitous ybiquitous left a comment

Choose a reason for hiding this comment

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

@kimulaco Thanks for creating the pull request.

I've commented on several refactoring suggestions. Please consider addressing them.

types/stylelint/index.d.ts Outdated Show resolved Hide resolved
lib/utils/FileCache.js Show resolved Hide resolved
lib/utils/FileCache.js Show resolved Hide resolved
lib/lintSource.js Outdated Show resolved Hide resolved
@ybiquitous
Copy link
Member

Due to #6358, there is a conflict. Can you please fix it?

lib/__tests__/cli.test.js Outdated Show resolved Hide resolved
lib/lintSource.js Outdated Show resolved Hide resolved
lib/lintSource.js Outdated Show resolved Hide resolved
types/stylelint/index.d.ts Outdated Show resolved Hide resolved
@Mouvedia
Copy link
Contributor

@kimulaco thanks for tackling the oldest non-upstream bug.

@ybiquitous ybiquitous changed the title Fix to lint when config is changed Fix cache refresh when config is changed Sep 26, 2022
Copy link
Member

@ybiquitous ybiquitous left a comment

Choose a reason for hiding this comment

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

@kimulaco Thank you for addressing the reviews.

I've left a suggestion for the test, but the code looks almost good enough to merge.

lib/__tests__/standalone-cache.test.js Show resolved Hide resolved
Copy link
Member

@ybiquitous ybiquitous left a comment

Choose a reason for hiding this comment

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

Thank you for dealing with several reviews. LGTM 👍🏼

@kimulaco
Copy link
Member Author

@ybiquitous Thanks for reviews. I understood stylelint better than ever.

Copy link
Member

@jeddy3 jeddy3 left a comment

Choose a reason for hiding this comment

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

@kimulaco Thank you for fixing the bug, LGTM!

@ybiquitous Thank you for the fantastic reviewing!

@jeddy3 jeddy3 merged commit 5be33b7 into stylelint:main Sep 27, 2022
@kimulaco kimulaco deleted the feat/issue-2908 branch September 28, 2022 14:50
jeddy3 pushed a commit that referenced this pull request Oct 5, 2022
…6393)

This type import was added in #6356, but during code review that PR was refactored to obviate the `file-entry-cache` types in the public API. The import was never removed. Keeping the import here results in [compilation errors](https://app.circleci.com/pipelines/github/palantir/blueprint/3156/workflows/9eda97ec-3a2f-4aa6-aa6c-89d5b65b8491/jobs/61694) for downstream consumers:

```
../../node_modules/stylelint/types/stylelint/index.d.ts:5:39 - error TS7016: Could not find a declaration file for module 'file-entry-cache'. '/home/circleci/project/node_modules/file-entry-cache/cache.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/file-entry-cache` if it exists or add a new declaration (.d.ts) file containing `declare module 'file-entry-cache';`

5  import type * as fileEntryCache from 'file-entry-cache'
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Fix .stylelintcache should be refreshed when config is changed
4 participants