deno fmt might not respects include/exclude options in workspace settings

[bartlomieju]

This is also a bug with deno fmt. I have a workspace setup where I have this fmt config:

  "fmt": {

    "include": ["*.ts", "*.tsx"],

    "exclude": ["*.d.ts", "./server/*"],

    "lineWidth": 120

  },

This still tries to process JS, CSS and even JSON files. However, when I also exclude the directory which contains those files (my Next JS build directory), it works fine. The include statement just shouldn't work with those files to begin with.

Originally posted by @dmint789 in #26308

[bartlomieju]

@dmint789 could you provide more info here? Do you have a workspace in your project? Is it open source and you could link a repo?

[dmint789]

I'll try to make a reproducible example soon. I do have an open source repo that could be used as an example, just need to fix a couple things first.

[dmint789]

@bartlomieju Okay, I have a reproducible example for both the deno lint bug and the deno fmt bug. The latter explicitly states Deno has panicked. This is a bug in Deno. Keep in mind that the fmt command is not deterministic (probably due to parallelism). First, clone this. Then follow these steps to reproduce:

cd client
deno task build # this will fail during linting, but that doesn't matter for this, it just has to create a `.next` directory
cd ..
deno fmt # this throws an error
deno lint # EPILEPSY WARNING! (not kidding). This gets stuck outputting garbage endlessly.

But then if you uncomment the exclude property in the root deno.jsonc file, it'll lint and format just fine.

[bartlomieju]

@dmint789 I tried your repro and here's what I found:

1. Running deno task build (after deno install) throws:

Task build next build
 ⨯ Failed to load next.config.js, see more info here https://nextjs.org/docs/messages/next-config-error

> Build error occurred
ReferenceError: module is not defined
    at file:///Users/ib/dev/cubingcontests.com/client/next.config.js:24:1

Which is a bug in itself. Which Deno version did you use to produce it? Changing module.exports = nextConfig to export default nextConfig fixes it.
2. I can reproduce a panic in deno fmt - again a couple problems here - .next directory probably shouldn't be checked by deno fmt by default and there's an actual panic formatting one file.
3. deno lint output is broken. Probably because it lints files in .next directory which most likely should be ignored by default.

That said - there's no deno.jsonc file in your reproduction repo - am I missing something here?

[dmint789]

@dmint789 I tried your repro and here's what I found:

1. Running deno task build (after deno install) throws:

Task build next build
 ⨯ Failed to load next.config.js, see more info here https://nextjs.org/docs/messages/next-config-error

> Build error occurred
ReferenceError: module is not defined
    at file:///Users/ib/dev/cubingcontests.com/client/next.config.js:24:1

Which is a bug in itself. Which Deno version did you use to produce it? Changing module.exports = nextConfig to export default nextConfig fixes it.
2. I can reproduce a panic in deno fmt - again a couple problems here - .next directory probably shouldn't be checked by deno fmt by default and there's an actual panic formatting one file.
3. deno lint output is broken. Probably because it lints files in .next directory which most likely should be ignored by default.

That said - there's no deno.jsonc file in your reproduction repo - am I missing something here?

Yes, because the reproducible example is in the specific branch and commit I linked.

[bartlomieju]

Okay, thanks. I was able to reproduce it on specific commit.

I created a simpler reproduction here: https://github.com/bartlomieju/deno-workspace-fmt-bug

[bartlomieju]

So I spent a few hours trying to figure out what's going wrong here and the answer is... this is working as expected at the moment.

There are two problems here:

1. The globs used here are like *.ts - this is only matching .ts files in the directory - it won't recursively descent into other directories. To do that you'd have to use **/*.ts
2. The include option only affects the directory it's defined in (in this case the root directory), while exclude option is "passed" down to all workspace members.

So in other words to achieve what you want, I'd suggest to add deno.json file in your client directory that looks like so:

{
  "fmt": {
    "include": ["**/*.ts", "**/*.tsx"],
    "exclude": ["**/*.d.ts"]
  }
}

This will only cause the .ts and .tsx files to be formatted and .js and .d.ts files will be left as they are.

I'm gonna keep this PR open for now as we will discuss if this is the behavior that should be in effect, it's quite confusing and not properly documented.