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

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

Open
bartlomieju opened this issue Oct 24, 2024 · 7 comments
Open
Assignees
Labels
deno fmt Related to the "deno fmt" subcommand or dprint needs discussion this topic needs further discussion to determine what action to take workspaces

Comments

@bartlomieju
Copy link
Member

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
Copy link
Member Author

@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?

@bartlomieju bartlomieju added needs info needs further information to be properly triaged deno fmt Related to the "deno fmt" subcommand or dprint labels Oct 24, 2024
@dmint789
Copy link

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
Copy link

dmint789 commented Oct 26, 2024

@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
Copy link
Member Author

@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
Copy link

@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
Copy link
Member Author

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 bartlomieju added bug Something isn't working correctly workspaces and removed needs info needs further information to be properly triaged labels Oct 28, 2024
@bartlomieju bartlomieju self-assigned this Oct 29, 2024
@bartlomieju
Copy link
Member Author

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.

@bartlomieju bartlomieju added needs discussion this topic needs further discussion to determine what action to take and removed bug Something isn't working correctly labels Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deno fmt Related to the "deno fmt" subcommand or dprint needs discussion this topic needs further discussion to determine what action to take workspaces
Projects
None yet
Development

No branches or pull requests

2 participants