Skip to content

feat(config): add ignoreOptions for node-ignore #6597

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

Merged
merged 5 commits into from
Nov 24, 2019
Merged

feat(config): add ignoreOptions for node-ignore #6597

merged 5 commits into from
Nov 24, 2019

Conversation

clarkdo
Copy link
Member

@clarkdo clarkdo commented Oct 20, 2019

Types of changes

  • New feature (a non-breaking change which adds functionality)

Description

Since node-ignore 4.x, it has support options as parameter of constructor, so this pr is adding the config from nuxt.config.js.

With this change, #6588 can be solved by:

// nuxt.config.js

export default {
  ignoreOptions: {
    ignorecase: false
  }
}

TODO:

  • Test
  • Doc

@clarkdo clarkdo requested a review from a team October 20, 2019 18:46
@codecov-io
Copy link

codecov-io commented Oct 21, 2019

Codecov Report

Merging #6597 into dev will increase coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #6597      +/-   ##
==========================================
+ Coverage   95.79%   95.83%   +0.03%     
==========================================
  Files          78       78              
  Lines        2713     2714       +1     
  Branches      702      702              
==========================================
+ Hits         2599     2601       +2     
+ Misses         99       98       -1     
  Partials       15       15
Flag Coverage Δ
#e2e 100% <ø> (ø) ⬆️
#fixtures 64.66% <100%> (+0.01%) ⬆️
#unit 91% <100%> (-1.29%) ⬇️
Impacted Files Coverage Δ
packages/config/src/config/_common.js 100% <ø> (ø) ⬆️
packages/builder/src/ignore.js 100% <100%> (ø) ⬆️
packages/vue-renderer/src/renderer.js 94.3% <0%> (+0.81%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 867b520...93eea85. Read the comment docs.

@atinux atinux changed the title feat(config): add ingoreOptions for node-ignore feat(config): add ignoreOptions for node-ignore Oct 22, 2019
@clarkdo clarkdo marked this pull request as ready for review October 30, 2019 18:05
@pi0 pi0 changed the title feat(config): add ignoreOptions for node-ignore feat(config): add ignoreOptions for node-ignore Nov 24, 2019
@pi0 pi0 merged commit 9ada4e6 into dev Nov 24, 2019
@pi0 pi0 deleted the feat/ignoreOptions branch November 24, 2019 12:28
@pi0 pi0 mentioned this pull request Nov 26, 2019
@chriscalo
Copy link
Contributor

Just tried this in the latest release 2.11.0 and it doesn't seem to be working.

nuxt.config.js:

export default {
  // .nuxtignore
  ignoreOptions: {
    ignorecase: false,
  },
};

.nuxtignore:

pages/**/[A-Z]*.vue

All pages/**/*.vue in my project seem to be ignored regardless of how they are named.

@clarkdo
Copy link
Member Author

clarkdo commented Dec 17, 2019

@chriscalo What is the version of ignore in you project? The options is supported from v4

@clarkdo
Copy link
Member Author

clarkdo commented Dec 17, 2019

And also can you try by exact path like pattern abc.vue doesn’t ignore ABC.vue.

@chriscalo
Copy link
Contributor

@clarkdo, thanks for the reply.

I'll check on the version when I get back to a computer, I haven't installed ignore myself. It's a dependency of Nuxt, so shouldn't Nuxt control the version rather than me?

And also can you try by exact path like pattern abc.vue doesn’t ignore ABC.vue.

Not sure what you mean by this. Can you say more? Where should I try exact paths?

@clarkdo
Copy link
Member Author

clarkdo commented Dec 17, 2019

Yes, it’s a dep from nuxt, but it may have conflict or hoisting if your other dep also has ignore package as a dep.
I mean use exact pattern in your ignore file like pages/abc.vue and verify it doesn’t ignore pages/ABC.vue in your repo.

@clarkdo
Copy link
Member Author

clarkdo commented Dec 17, 2019

I’m sorry that I’m in holiday. I’ll check this as soon as I’m back.

@chriscalo
Copy link
Contributor

Here's what I found in my yarn.lock file:

ignore@^5.1.1, ignore@^5.1.4:

@chriscalo
Copy link
Contributor

I mean use exact pattern in your ignore file like pages/abc.vue and verify it doesn’t ignore pages/ABC.vue in your repo.

Thanks, will try that.

I’m sorry that I’m in holiday. I’ll check this as soon as I’m back.

Enjoy! Please don't rush on my account. This refactoring can wait. Just wanted to point out that it didn't seem to be working. Happy to provide more debug info if that's useful.

@chriscalo
Copy link
Contributor

I think I may have figured out the problem: my macOS HD is case insensitive, which I'm guessing means the ignoreCase won't work as expected. I may have to find some other way of solving my problem. Apologies for the runaround on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants