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

feat: add chokidarOptions to override the default ignored #5223

Closed
wants to merge 7 commits into from

Conversation

4-1-1
Copy link

@4-1-1 4-1-1 commented Oct 8, 2021

Description

Additional context


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@4-1-1 4-1-1 closed this Oct 8, 2021
@4-1-1 4-1-1 reopened this Oct 8, 2021
@4-1-1 4-1-1 changed the title add chokidarOptions to override the default ignored feat: add chokidarOptions to override the default ignored Oct 8, 2021
Copy link
Member

@Shinigami92 Shinigami92 left a comment

Choose a reason for hiding this comment

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

I'm also missing a description in the PR. Could you explain why this is needed?
Does it resolve an issue?
May we want to merge chokidarOptions and watchOptions to one override object?

packages/vite/types/chokidar.d.ts Outdated Show resolved Hide resolved
/**
* use to override the default ignored
*/
chokidarOptions?: any
Copy link
Member

Choose a reason for hiding this comment

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

Missing type, is there really no typedef exported by chokidar that could be reused?

packages/vite/types/chokidar.d.ts Outdated Show resolved Hide resolved
4-1-1 and others added 2 commits October 8, 2021 14:20
Co-authored-by: Shinigami <chrissi92@hotmail.de>
Co-authored-by: Shinigami <chrissi92@hotmail.de>
@patak-dev
Copy link
Member

You can use server.watch to pass these options already. Did you find an issue doing so?

@4-1-1
Copy link
Author

4-1-1 commented Oct 8, 2021

You can use server.watch to pass these options already. Did you find an issue doing so?

I want to override the ignored option, you can check the code, vite provide a ignored option to chockidar , and can't to be override , I need to watch an package in node_modules. @patak-js

@patak-dev
Copy link
Member

Maybe we should replace the default ignored if the user specify one. We should check if this was not done already for a reason

@antfu
Copy link
Member

antfu commented Oct 8, 2021

Agree that we should allow users to override when provided.

@4-1-1
Copy link
Author

4-1-1 commented Oct 8, 2021

@patak-js @antfu Yes, Maybe check first is better, but it's not compatible

@patak-dev
Copy link
Member

What do you mean? Couldnt we check if ignored is present and use that instead of our default?

@4-1-1
Copy link
Author

4-1-1 commented Oct 8, 2021

What do you mean? Couldnt we check if ignored is present and use that instead of our default?

If Someone use vitejs , and provide an ignored option before, it will not ignored node_modules and .git .
if you use new option to do lt , they will be always ok @patak-js

@antfu
Copy link
Member

antfu commented Oct 8, 2021

Yes, this could be a breaking change. But I guess a pretty minor one that we could possibly fit in 2.7

@4-1-1
Copy link
Author

4-1-1 commented Oct 8, 2021

Ok, i'm waiting 🤔

@bluwy
Copy link
Member

bluwy commented Oct 8, 2021

I suppose this fixes #5023, we should have it on the PR description

@4-1-1
Copy link
Author

4-1-1 commented Oct 8, 2021

I suppose this fixes #5023, we should have it on the PR description

yes

@Shinigami92 Shinigami92 added the p2-nice-to-have Not breaking anything but nice to have (priority) label Oct 8, 2021
@haoqunjiang
Copy link
Member

Negated glob patterns have higher priority than positive ones.
So configurations like ignored: ['!**/node_modules/**'] would suffice. Let's document it instead of adding an option.

@haoqunjiang haoqunjiang closed this Oct 8, 2021
@4-1-1
Copy link
Author

4-1-1 commented Oct 9, 2021

@sodatea
image
it's not working. Is there a way to disable .vite cache, the package don't update before i remove node_modules/.vite

@haoqunjiang
Copy link
Member

@4-1-1 See the example in the corresponding documentation PR: https://github.com/vitejs/vite/pull/5239/files

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p2-nice-to-have Not breaking anything but nice to have (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants