Deno fmt should ignore the node_modules directory?

[uki00a]

• Deno: v0.36.0

When running deno fmt without arguments, files in the node_modules directory are formatted.

$ pwd
/home/uki00a/work/denofmt-test

$ yarn list
yarn list v1.21.1
└─ [email protected]
Done in 0.08s.

$ find .
.
./node_modules
./node_modules/.yarn-integrity
./node_modules/ansi-regex
./node_modules/ansi-regex/index.js
./node_modules/ansi-regex/license
./node_modules/ansi-regex/package.json
./node_modules/ansi-regex/readme.md
./node_modules/ansi-regex/index.d.ts
./package.json
./yarn.lock

$ deno fmt --check
Found 1 not formatted file

$ deno fmt
/home/uki00a/work/denofmt-test/node_modules/ansi-regex/index.js

[lucacasonato]

I don't think we should disable that. Deno doesn't do magic, and this would be arbitrary magic.

[Soremwar]

Deno is agnostic of node. To Deno that's just a folder full of Scripts

[kitsonk]

I agree... if you tell Deno to format stuff in a path, it will... format stuff in that path.

Potentially, I could see how repeating some sort of glob path and having the ability to ignore, like .gitignore could be beneficial, but some sort of #3179 would be the only suitable solution I think.

[bartossh]

node_modules to Deno are not what they are to Node. I thinks it is one of the basics of Deno philosophy to do not rely of any of that which node_modules are. You could do what you have done for any folder with scripts.

[mathiasrw]

Why would you put a node_modules folder in a deno project?

[keroxp]

I think the default deno fmt file range is too open. Without argument, every single .ts .js .jsx .tsx file in current working directory and its subtrees are included .

This is totally different behavior from other well-known formatters. prettier doesn't have default behavior for empty argument and go fmt only formats files in current directory.

[kitsonk]

IMO, without an argument, only support the current directory, makes sense and there is precedence.

[hayd]

👍 to fmt (and test) following .gitignore (perhaps .npmignore too 😳 ). Unfortunately external (non-deno) tools do like to dump stuff into node_modules. I don't agree this is "magic" in the same the same way as node-style module resolution in run is magic. This is pragmatic and could be well-defined in the subcommand.

[kitsonk]

👎 for following .gitignore and .npmignore. Deno shouldn't look at anything it doesn't own. Tightly coupling to another ecosystem is illogical.

[hayd]

This "magic" argument makes sense for run but why developer tools like fmt and test?

Deno shouldn't look at anything it doesn't own.

These tools/subcommands already look in the entire directory!

[kitsonk]

This "magic" argument makes sense for run but why developer tools like fmt and test?

Magic resolution or behaviour is bad, no matter the context. The easiest thing to do is limit to the current directory when no argument is passed. Then there is no surprises, but if someone says "go look in here" then Deno will. Simple.

These tools/subcommands already look in the entire directory!

You are missing my point. Deno shouldn't interpret/couple to something it doesn't own the semantics of.

[mathiasrw]

Deno shouldn't interpret/couple to something it doesn't own the semantics of.

I agree. If you choose to place a node_module folder in a deno project it must be treated like everything else. I do see it as the natural behavour what its doing now where deno fmt fixes everything.

Input regarding .gitignore files: Its much faster to run the files tracked by git instead of finding all the files and removing the .gitignore ones.

If it can be any inspiration for anyone you get the complete list of files checked in with git ls-tree --full-tree --name-only -r HEAD and it can be used like

git ls-tree --full-tree --name-only -r HEAD | grep -E '\\.(scss|css|js|ts|vue|html|json|jsx|tsx)$' | xargs prettier --write

(no, im not suggesting we start using prettier - its just an example 😄 )

[uki00a]

I'm sorry for late reply and thanks for many comments!

First of all, I was trying to use npm modules with Node polyfill (e.g. like this: https://github.com/davidbailey00/deno-babel-demo).

When I run deno fmt at that time, I noticed that more files are formatted than expected (This is due to the large number of files in the node_modules directory).

So IMO I think it would be useful to have a way to ignore certain files 😄

[ry]

I'm not particularly against ignoring node_modules folder. The Deno philosophy is to not have special logic for module resolution, but I'm not sure that needs to extend to "deno fmt". As far as I'm concerned it could also read .prettierignore.

[Soremwar]

@ry I semi-agree with that because currently Node is the only big platform that handles JS modules. However in the future that situation might change and in that case it would be convenient for the user to specify which folders may require Dprint formatting, instead of hardcoding this folders in Deno.

[mathiasrw]

Respecting .prettierignore seems to me like the most sound solution.

[Skillz4Killz]

I think this should extend beyond the node_modules folder as documentation could be kept in the same repo and to make it ignore a directory is much better than only node_modules. For example, ignoring the docs directory in the repo would be extremely helpful. IMO this request should encompass more than just node_modules but allow any directory to be ignored

[mathiasrw]

I like what is going on in #3827

Solving this issue could also be done by adding a parameter like --dot-ignore and then specify a file with the same content as .prettierignore or .npmignore (or .fmtignore)

[Spoonbender]

I don't think deno fmt should magically ignore things, but there should be some option for excluding paths based on pattern, be it via command line argument or a configuration file of some sort (which will be either specifyable in cmd args or implicitly detected).

I think higher level tools, like IDEs, should be able to utilize deno fmt and stich it with logic relevant to them, i.e. the IDE will parse the .gitignore file and pass the exclusions list to deno fmt.

[Soremwar]

@lucacasonato Just saw you were cleaning issues. I think this can be closed since --ignore was implemented.

[hayd]

This is still a massive pain e.g. in a project that happens to include several node projects (or examples) where the node_modules are nested in deeper subdirectories (so not clear how you even pass that to --ignore).
deno fmt gets completely overwhelmed (can takes tens of minutes parsing junk).

I don't understand the use case where you would want to format a node_modules directory.
Yes it's a (minor!) "special case" but it is simply not unusual* for node_modules directories to exist in js projects.

* a fact of life for the forseeable?

Edit: Also in light of #12295.

[fibo]

This issue can be closed! (Thanks to deno devs)

just add to your config file

  "fmt": {
    "files": {
      "exclude": ["node_modules"]
    }
  },

Same goes for lint, see reference here https://deno.land/[email protected]/getting_started/configuration_file

[nayeemrmn]

We should make this default when --compat is passed.

[bartlomieju]

We should make this default when --compat is passed.

--compat is only available to "runtime" subcommands. This needs more discussion as it might imply more work (similar for deno lint)

[nayeemrmn]

Yeah we'd have to give the flag to fmt as well or make it global, is there a real obstacle that I'm missing?

[bartlomieju]

Yeah we'd have to give the flag to fmt as well or make it global, is there a real obstacle that I'm missing?

I guess the only obstacle is what are expectations for this flag.

[hayd]

IMO It should be the default.

I don't understand the use case where you would want to format a node_modules directory.

Still stands. No-one wants to do this (same with lint).

[nayeemrmn]

I don't understand the use case where you would want to format a node_modules directory.

There are probably lots of 'known' things you would never want to be included in any given operation, but IMO tools that implicitly reach around for things beyond their own concerns by default are not well composable. But we now have --compat precisely for Node interop, it would be perfect to skip node_modules only for deno fmt --compat.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

[hayd]

Given there is now precedent in #14138 can this be re-opened ? 😉

[bartlomieju]

@dsherret let's just do it now?

[dsherret]

I think so. It would be updating this line and changing the function name:

deno/cli/tools/fmt.rs

Line 726 in 2724235

path.components().any(|c| c.as_os_str() == ".git")

[bartlomieju]

@dsherret why "maybe 2.0" label? I think we can just land it in the next minor.

[dsherret]

@bartlomieju yeah, that's what I think. I removed that label (didn't add it) 😄

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.