Ignoring of node_modules doesn't work on certain packages

[ColinMorris83]

It doesn't look like ignoring node_modules works. Have gone back trying previous versions, and it looks to have been introduced in 0.34.0 as 0.33.0 is working ok.

package.json script:

"lint:markdown": "markdownlint '**/*.md' --ignore node_modules"

Results in errors coming from node_modules:

> markdownlint '**/*.md' --ignore node_modules

node_modules/@optimizely/js-sdk-logging/CHANGELOG.MD:1 MD022/blanks-around-headings Headings should be surrounded by blank lines [Expected: 1; Actual: 0; Below] [Context: "# Changelog"]
node_modules/@optimizely/js-sdk-logging/CHANGELOG.MD:7 MD022/blanks-around-headings Headings should be surrounded by blank lines [Expected: 1; Actual: 0; Below] [Context: "## [Unreleased]"]
node_modules/@optimizely/js-sdk-logging/CHANGELOG.MD:12 MD022/blanks-around-headings Headings should be surrounded by blank lines [Expected: 1; Actual: 0; Below] [Context: "### Bug Fixes"]

[DavidAnson]

I can't try this from my phone, but version 0.34.0 was released about 10 months ago and this is pretty fundamental to be broken that long without anyone else noticing. Current version is 0.39.0 - could you please try that and confirm the version of this and Node.js (both via --version on the command line) that you are using?

[ColinMorris83]

Markdownlint version: 0.39.0
Node version: v20.11.0

[nschonni]

@DavidAnson maybe related to the changes to support scoped packages e785e42 ?

[DavidAnson]

I do not observe the reported behavior. Here's what I did in a new GitHub Codespace for this repo after deleting all files. Note that I explicitly included the package shown in the example above. Please review and provide specific steps to reproduce the behavior you observe, ideally by pointing to a repository/commit that demonstrates the problem.

@DavidAnson ➜ /workspaces/markdownlint-cli (master) $ node --version
v20.11.0
@DavidAnson ➜ /workspaces/markdownlint-cli (master) $ npm --version
10.2.4
@DavidAnson ➜ /workspaces/markdownlint-cli (master) $ ls -a
.  ..  .git  package.json
@DavidAnson ➜ /workspaces/markdownlint-cli (master) $ cat package.json
{
  "dependencies": {
    "markdownlint-cli": "0.39.0",
    "@optimizely/js-sdk-logging": "0.3.1"
  },
  "scripts": {
    "version": "markdownlint --version",
    "include": "markdownlint '**/*.md'",
    "exclude": "markdownlint '**/*.md' --ignore node_modules"
  }
}
@DavidAnson ➜ /workspaces/markdownlint-cli (master) $ npm install

...
@DavidAnson ➜ /workspaces/markdownlint-cli (master) $ npm run version

> version
> markdownlint --version

0.39.0
@DavidAnson ➜ /workspaces/markdownlint-cli (master) $ npm run include

> include
> markdownlint '**/*.md'

node_modules/@isaacs/cliui/README.md:71:1 MD033/no-inline-html Inline HTML [Element: img]
...
node_modules/@optimizely/js-sdk-logging/README.md:15 MD012/no-multiple-blanks Multiple consecutive blank lines [Expected: 1; Actual: 2]
...
node_modules/wrap-ansi/readme.md:90:1 MD010/no-hard-tabs Hard tabs [Column: 1]

@DavidAnson ➜ /workspaces/markdownlint-cli (master) $ npm run exclude

> exclude
> markdownlint '**/*.md' --ignore node_modules

@DavidAnson ➜ /workspaces/markdownlint-cli (master) $ 

[ColinMorris83]

Hi,
I setup a new repo: https://github.com/ColinMorris83/markdownlint-testing
I have tried this with both Windows and on Macbook.

On both, the include script lists hundreds of errors covering multiple packages.
On both, the exclude script only lists errors from node_modules/@optimizely

[tyrann0us]

@ColinMorris83, FYI, I created a PR to ignore node_modules/ by default: #460.

[DavidAnson]

@ColinMorris83, thanks, I will try tonight via Codespaces. To be clear, you originally said that ignore of node_modules didn't work (at all?), but your latest comment seems to say it's only this one package within node_modules that's not being ignored in your scenario? (If so, that rules out certain possibilities.)

[ColinMorris83]

@ColinMorris83, thanks, I will try tonight via Codespaces. To be clear, you originally said that ignore of node_modules didn't work (at all?), but your latest comment seems to say it's only this one package within node_modules that's not being ignored in your scenario? (If so, that rules out certain possibilities.)

Hey, yeah it looks like it does mostly work, but doesn't work for this particular package. I initially thought perhaps it was because it started with an @ but I don't think that is the cause because there are other packages also starting with that which do show errors when not ignoring node_modules, but it's only the optimizely one still having errors when node_modules is being ignored.

[DavidAnson]

Codespaces demonstrates the expected/correct behavior for your repo. I created a new Codespace via the GitHub UI, let it automatically install dependencies, then ran the following in the default bash terminal:

@DavidAnson ➜ /workspaces/markdownlint-testing (main) $ npm run exclude

> [email protected] exclude
> markdownlint "**/*.md" --ignore node_modules

@DavidAnson ➜ /workspaces/markdownlint-testing (main) $ 

Whatever's happening to you seems to be unique to your configuration. Codespaces are nice because they provide a clean, reproducible environment. Please see if you can update your repository to reproduce the issue - and you may find what's special about your environment in the process.

[ColinMorris83]

Thanks, yeah if I do a codespace off the repo, it works ok. However, I've got 2 other people to also try checking out the repo and running locally, and they are getting the same issue as me. I've also got it happening on both my work macbook and my home PC on windows.

What does work is changing the script to this:
"exclude": "markdownlint \"**/*.md\" --ignore \"node_modules/**\""

So I'll use that for now

[DavidAnson]

Great. FYI, because we agree this does not reproduce with the repository you created (via Codespaces) and because there is a unit test for directory ignore that passes on Mac, Ubuntu, and Windows (

markdownlint-cli/test/test.js

Lines 214 to 223 in ca05bed

	test('glob linting with failing files has fewer errors when ignored by dir', async t => {
	try {
	await execa('../markdownlint.js', ['--config', 'test-config.json', '**/*.md', '--ignore', 'subdir-incorrect'], {stripFinalNewline: false});
	t.fail();
	} catch (error) {
	t.is(error.stdout, '');
	t.is(error.stderr.match(errorPattern).length, 8);
	t.is(error.exitCode, 1);
	}
	});

) and because no one else is reporting a problem with this common scenario, I intend to close the issue soon. I will try a couple more things myself but if they don't produce the behavior you describe, this seems to be something about how your environment is configured. If you find out what that is someday, please update this issue. (One thing you might try is using the Docker image because that should be more isolated from environmental weirdness.) Thank you.

[ColinMorris83]

I think I spot what's occurring now:

The joined path when ignore is set to just node_modules is this:

node_modules/**/*.{md,markdown}

And if you look at the errors:
node_modules/@optimizely/js-sdk-logging/CHANGELOG.MD:1 MD022

The extension is capital MD

I tried renaming that to lowercase md, and that resolves it.

So looks to be a casing issue

[DavidAnson]

Great find! I will look into this later when I have time. I'm not sure how that file got the "wrong" casing on your machine, but I don't see any reason why the ignore pattern should be case-sensitive. I should be able to change that safely so this won't be a problem for you again.

[ColinMorris83]

It looks like those packages that have it with capitals have not been published in a long time, but at their last publish, they contained the capitals. e.g. https://github.com/optimizely/javascript-sdk/tree/3.2.x/packages/utils

If it helps, I tried adding in nocase: true to the minimatch match options and that seems to do the trick, but there may be better/other ways of handling it

return previousResults.filter(fileInfo => matcher.match(fileInfo.absolute, { nocase: true })).map(fileInfo => fileInfo.original);

Just did a bit of further digging, looks like they did correct the file naming: optimizely/javascript-sdk@5d526bd but haven't actually published again to npm, so we're stuck with it being incorrectly named.