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

📝 Biome doesn't format .jsonc file with trailing commas #1659

Closed
1 task done
Sec-ant opened this issue Jan 25, 2024 · 7 comments · Fixed by #1944
Closed
1 task done

📝 Biome doesn't format .jsonc file with trailing commas #1659

Sec-ant opened this issue Jan 25, 2024 · 7 comments · Fixed by #1944
Assignees
Labels
A-Formatter Area: formatter S-Bug-confirmed Status: report has been confirmed as a valid bug

Comments

@Sec-ant
Copy link
Member

Sec-ant commented Jan 25, 2024

Environment information

❯ npx biome rage
CLI:
  Version:                      1.5.3
  Color support:                true

Platform:
  CPU Architecture:             x86_64
  OS:                           linux

Environment:
  BIOME_LOG_DIR:                unset
  NO_COLOR:                     unset
  TERM:                         "xterm-256color"
  JS_RUNTIME_VERSION:           "v21.6.0"
  JS_RUNTIME_NAME:              "node"
  NODE_PACKAGE_MANAGER:         "npm/10.2.4"

Biome Configuration:
  Status:                       Loaded successfully
  Formatter disabled:           false
  Linter disabled:              false
  Organize imports disabled:    false
  VCS disabled:                 true

Workspace:
  Open Documents:               0

Configuration

{
  "$schema": "./node_modules/@biomejs/biome/configuration_schema.json",
  "formatter": {
    "enabled": true
  },
  "overrides": [
    {
      "include": ["*.jsonc"],
      "json": {
        "parser": {
          "allowComments": true,
          "allowTrailingCommas": true
        }
      }
    }
  ]
}

Playground link

https://github.com/Sec-ant/biome-jsonc-formatting-bug-repro

Code of Conduct

  • I agree to follow Biome's Code of Conduct
@Sec-ant
Copy link
Member Author

Sec-ant commented Jan 25, 2024

emmm, I seem to mess up the CSB link. You can test it with a .jsonc file like this:

{"foo":          "bar",
}

A GitHub repo to repro is provided.

@Sec-ant
Copy link
Member Author

Sec-ant commented Jan 25, 2024

Also, FWIW, there's a discussion around the trailing comma formatting behavior of jsonc files in Prettier recently: prettier/prettier#15956

I'm curious about what position will Biome take in this question?

@ematipico
Copy link
Member

Personally, I would like to not deviate too much from the specs. Fortunately, we offer the option to allow trailing commas. That should help. Unless there's a bug when you enable parser.json.allowTrailingCommas

@Sec-ant
Copy link
Member Author

Sec-ant commented Jan 31, 2024

Unless there's a bug when you enable parser.json.allowTrailingCommas

Yes that's what this issue is about. But I have trouble sharing the link to my repro in the CSB devbox (the link always jumps to a new template, I don't know why), so I created a github repo here: https://github.com/Sec-ant/biome-jsonc-formatting-bug-repro Thanks for checking this.

@Sec-ant
Copy link
Member Author

Sec-ant commented Jan 31, 2024

Personally, I would like to not deviate too much from the specs. Fortunately, we offer the option to allow trailing commas. That should help.

Ok, let me rephrase my question in a more specific way: If I configure biome in a way that trailing commas are allowed for all the .jsonc files, and I have a .jsonc file that do not have trailing commas (or only has some trailing commas over the place, but not everywhere) that need to be formatted, will trailing commas be added to (or deleted from, or kept untouched in) the file after formatting?

@Sec-ant
Copy link
Member Author

Sec-ant commented Feb 28, 2024

I not very confident with the codebase but I think the switch to support it is here, is this planned to be fixed @ematipico ?

.with_trailing_separator(TrailingSeparator::Disallowed)

#926 seems also related.

@ematipico ematipico added the S-Bug-confirmed Status: report has been confirmed as a valid bug label Feb 28, 2024
@ematipico ematipico self-assigned this Feb 28, 2024
@ematipico ematipico added the A-Formatter Area: formatter label Feb 28, 2024
@ematipico
Copy link
Member

Working on this issue. It seems that our tests weren't optimal, and I can see that this isn't working at all.

Sorry for neglecting the issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Formatter Area: formatter S-Bug-confirmed Status: report has been confirmed as a valid bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants