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 check" result differs according to path depth #2080

Closed
1 task done
bc-m opened this issue Mar 13, 2024 · 14 comments · Fixed by #2186
Closed
1 task done

💅 "biome check" result differs according to path depth #2080

bc-m opened this issue Mar 13, 2024 · 14 comments · Fixed by #2186
Assignees
Labels
A-Project Area: project S-Bug-confirmed Status: report has been confirmed as a valid bug

Comments

@bc-m
Copy link

bc-m commented Mar 13, 2024

Environment information

CLI:
  Version:                      1.6.1
  Color support:                true

Platform:
  CPU Architecture:             aarch64
  OS:                           macos

Environment:
  BIOME_LOG_DIR:                unset
  NO_COLOR:                     unset
  TERM:                         "xterm-256color"
  JS_RUNTIME_VERSION:           "v20.11.1"
  JS_RUNTIME_NAME:              "node"
  NODE_PACKAGE_MANAGER:         unset

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

Linter:
  Recommended:                  true
  All:                          false
  Rules:                        complexity/noExcessiveCognitiveComplexity = {"level":"error","options":{"maxAllowedComplexity":45}}
                                complexity/noUselessSwitchCase = "off"
                                complexity/noVoid = "error"
                                correctness/noUnusedVariables = "error"
                                correctness/useExhaustiveDependencies = "off"
                                nursery/noNodejsModules = "error"
                                performance/noAccumulatingSpread = "off"
                                style/noInferrableTypes = "off"
                                style/useEnumInitializers = "off"
                                suspicious/noConsoleLog = "error"

Workspace:
  Open Documents:               0

Playground link

https://biomejs.dev/playground/?lineWidth=120&code=LwAvACAAcgBlAHMAdQBsAHQAIABhAGYAdABlAHIAIABiAGkAbwBtAGUAIABmAG8AcgBtAGEAdAAKAGMAbwBuAHMAdAAgAGcAZQB0AFcAcgBhAHAAcABlAHIAIAA9ACAAKABvAHAAdABpAG8AbgBzADoAIABHAGUAdABXAHIAYQBwAHAAZQByAFAAcgBvAHAAcwAgAD0AIAB7AH0AKQAgAD0APgAgAHsACgAJAGMAbwBuAHMAdAAgAHsAIABlAG4AdAByAHkALAAgAGEAYwB0AGkAdgBlAEQAaQBhAGwAbwBnACwAIABtAHUAbAB0AGkAUwBlAGwAZQBjAHQAaQBvAG4AIAB9ACAAPQAgAG8AcAB0AGkAbwBuAHMAOwAKAAkAcgBlAHQAdQByAG4AIAAoAHsAIABjAGgAaQBsAGQAcgBlAG4AIAB9ACkAOgAgAFIAZQBhAGMAdABFAGwAZQBtAGUAbgB0ACAAPQA%2BACAAKAAKAAkACQA8AFQAZQBzAHQAQgBhAHMAZQBQAHIAbwB2AGkAZABlAHIAPgAKAAkACQAJADwARABpAGEAbABvAGcAQwBvAG4AdABlAHgAdAAuAFAAcgBvAHYAaQBkAGUAcgAgAHYAYQBsAHUAZQA9AHsAewAgAGMAbABvAHMAZQBEAGkAYQBsAG8AZwA6ACAAYwBsAG8AcwBlAEQAaQBhAGwAbwBnAE0AbwBjAGsALAAgAGEAYwB0AGkAdgBlAEQAaQBhAGwAbwBnACAAfQB9AD4ACgAJAAkACQAJAHsAbQB1AGwAdABpAFMAZQBsAGUAYwB0AGkAbwBuACAAPwAgACgACgAJAAkACQAJAAkAYwBoAGkAbABkAHIAZQBuAAoACQAJAAkACQApACAAOgAgACgACgAJAAkACQAJAAkAPABFAG4AdAByAHkAQwBvAG4AdABlAHgAdAAuAFAAcgBvAHYAaQBkAGUAcgAgAHYAYQBsAHUAZQA9AHsAewAgAGUAbgB0AHIAeQAsACAAcwBlAHQARQBuAHQAcgB5ADoAIAB2AGkALgBmAG4AKAApACAAfQB9AD4AewBjAGgAaQBsAGQAcgBlAG4AfQA8AC8ARQBuAHQAcgB5AEMAbwBuAHQAZQB4AHQALgBQAHIAbwB2AGkAZABlAHIAPgAKAAkACQAJAAkAKQB9AAoACQAJAAkAPAAvAEQAaQBhAGwAbwBnAEMAbwBuAHQAZQB4AHQALgBQAHIAbwB2AGkAZABlAHIAPgAKAAkACQA8AC8AVABlAHMAdABCAGEAcwBlAFAAcgBvAHYAaQBkAGUAcgA%2BAAoACQApADsACgB9ADsACgAKAC8ALwAgAHIAZQBzAHUAbAB0ACAAYQBmAHQAZQByACAAYgBpAG8AbQBlACAAYwBoAGUAYwBrAAoAYwBvAG4AcwB0ACAAZwBlAHQAVwByAGEAcABwAGUAcgAyACAAPQAgACgAbwBwAHQAaQBvAG4AcwA6ACAARwBlAHQAVwByAGEAcABwAGUAcgBQAHIAbwBwAHMAIAA9ACAAewB9ACkAIAA9AD4AIAB7AAoAIAAgAGMAbwBuAHMAdAAgAHsAIABlAG4AdAByAHkALAAgAGEAYwB0AGkAdgBlAEQAaQBhAGwAbwBnACwAIABtAHUAbAB0AGkAUwBlAGwAZQBjAHQAaQBvAG4AIAB9ACAAPQAgAG8AcAB0AGkAbwBuAHMAOwAKACAAIAByAGUAdAB1AHIAbgAgACgAewAgAGMAaABpAGwAZAByAGUAbgAgAH0AKQA6ACAAUgBlAGEAYwB0AEUAbABlAG0AZQBuAHQAIAA9AD4AIAAoAAoAIAAgACAAIAA8AFQAZQBzAHQAQgBhAHMAZQBQAHIAbwB2AGkAZABlAHIAPgAKACAAIAAgACAAIAAgADwARABpAGEAbABvAGcAQwBvAG4AdABlAHgAdAAuAFAAcgBvAHYAaQBkAGUAcgAgAHYAYQBsAHUAZQA9AHsAewAgAGMAbABvAHMAZQBEAGkAYQBsAG8AZwA6ACAAYwBsAG8AcwBlAEQAaQBhAGwAbwBnAE0AbwBjAGsALAAgAGEAYwB0AGkAdgBlAEQAaQBhAGwAbwBnACAAfQB9AD4ACgAgACAAIAAgACAAIAAgACAAewBtAHUAbAB0AGkAUwBlAGwAZQBjAHQAaQBvAG4ACgAgACAAIAAgACAAIAAgACAAIAAgAD8AIABjAGgAaQBsAGQAcgBlAG4ACgAgACAAIAAgACAAIAAgACAAIAAgADoAIAA8AEUAbgB0AHIAeQBDAG8AbgB0AGUAeAB0AC4AUAByAG8AdgBpAGQAZQByACAAdgBhAGwAdQBlAD0AewB7ACAAZQBuAHQAcgB5ACwAIABzAGUAdABFAG4AdAByAHkAOgAgAHYAaQAuAGYAbgAoACkAIAB9AH0APgB7AGMAaABpAGwAZAByAGUAbgB9ADwALwBFAG4AdAByAHkAQwBvAG4AdABlAHgAdAAuAFAAcgBvAHYAaQBkAGUAcgA%2BAH0ACgAgACAAIAAgACAAIAA8AC8ARABpAGEAbABvAGcAQwBvAG4AdABlAHgAdAAuAFAAcgBvAHYAaQBkAGUAcgA%2BAAoAIAAgACAAIAA8AC8AVABlAHMAdABCAGEAcwBlAFAAcgBvAHYAaQBkAGUAcgA%2BAAoAIAAgACkAOwAKAH0AOwA%3D

Expected result

biome check should format file like biome format.

Code of Conduct

  • I agree to follow Biome's Code of Conduct
@bc-m bc-m changed the title 💅 biome check result differs from biome format 💅 "biome check" result differs from "biome format" Mar 13, 2024
@ematipico
Copy link
Member

I am not sure what the issue is. Can you please provide more information? The playground alone doesn't really help

@ematipico ematipico added the S-Needs response Status: await response from OP label Mar 13, 2024
@bc-m
Copy link
Author

bc-m commented Mar 13, 2024

Thanks for your fast response. I try it with a step-by-step example...

Step 1: I have a file with following content:

const getWrapper = (options: GetWrapperProps = {}) => {
  const { entry, activeDialog, multiSelection } = options;
  return ({ children }): ReactElement => (
    <TestBaseProvider>
      <DialogContext.Provider value={{ closeDialog: closeDialogMock, activeDialog }}>
        {multiSelection ? (
          children
        ) : (
          <EntryContext.Provider value={{ entry, setEntry: vi.fn() }}>{children}</EntryContext.Provider>
        )}
      </DialogContext.Provider>
    </TestBaseProvider>
  );
};

Step 2: Using "biome check ./src --apply" the file was changes to:

const getWrapper2 = (options: GetWrapperProps = {}) => {
  const { entry, activeDialog, multiSelection } = options;
  return ({ children }): ReactElement => (
    <TestBaseProvider>
      <DialogContext.Provider value={{ closeDialog: closeDialogMock, activeDialog }}>
        {multiSelection
          ? children
          : <EntryContext.Provider value={{ entry, setEntry: vi.fn() }}>{children}</EntryContext.Provider>}
      </DialogContext.Provider>
    </TestBaseProvider>
  );
};

Step 3: Using "biome check ./src/subdir --apply" or "biome check ./src/path/to/file.tsx" (also used by JetBrains IntellJ Biome-plugin on save) the files changes back to:

const getWrapper = (options: GetWrapperProps = {}) => {
  const { entry, activeDialog, multiSelection } = options;
  return ({ children }): ReactElement => (
    <TestBaseProvider>
      <DialogContext.Provider value={{ closeDialog: closeDialogMock, activeDialog }}>
        {multiSelection ? (
          children
        ) : (
          <EntryContext.Provider value={{ entry, setEntry: vi.fn() }}>{children}</EntryContext.Provider>
        )}
      </DialogContext.Provider>
    </TestBaseProvider>
  );
};

Its looks like "biome check ./src/subdir" prefers:

{multiSelection ? (
  children
) : (
  <EntryContext.Provider value={{ entry, setEntry: vi.fn() }}>{children}</EntryContext.Provider>
)}

while "biome check ./src" prefers:

{multiSelection
  ? children
  : <EntryContext.Provider value={{ entry, setEntry: vi.fn() }}>{children}</EntryContext.Provider>}

Edit: Adjusted examples based on new knowledge from #2080 (comment)

@Niznikr
Copy link

Niznikr commented Mar 13, 2024

Running into the same issue 😅

@ematipico
Copy link
Member

Thank you @bc-m! That's definitely a weird issue

@ematipico ematipico added A-Project Area: project S-Bug-confirmed Status: report has been confirmed as a valid bug and removed S-Needs response Status: await response from OP labels Mar 13, 2024
@bc-m
Copy link
Author

bc-m commented Mar 14, 2024

It gets even stranger...

When i execute biome check ./src --apply the result is:

{multiSelection
  ? children
  : <EntryContext.Provider value={{ entry, setEntry: vi.fn() }}>{children}</EntryContext.Provider>}

When i execute biome check ./src/files/components/DeleteDialog/DeleteDialog.spec.tsx --apply, biome check ./src/files/components/DeleteDialog --apply or even biome check ./src/files --apply the result is:

{multiSelection ? (
  children
) : (
  <EntryContext.Provider value={{ entry, setEntry: vi.fn() }}>{children}</EntryContext.Provider>
)}

This is why our git-commit-hook and the intellj biome-plugin change the file to the second format. Then our CI fails because it expects the first format.

@bc-m bc-m changed the title 💅 "biome check" result differs from "biome format" 💅 "biome check" result differs according to path depth Mar 14, 2024
@mrljsh
Copy link

mrljsh commented Mar 18, 2024

When dealing with monorepos, it seems that the command biome check . --apply uses the root biome.json configuration for formatting exclusively, and neglects to apply child configurations. However, the check on save feature appears to function properly.

Regarding the issue you’re experiencing, may I assume that you aren’t utilizing a monorepo or multiple configuration files? Could you please share your biome configuration with us?

@ematipico
Copy link
Member

@mrljsh Biome doesn't support child configurations. Feel free to check the discussions to add your take about monorepos.

@mrljsh
Copy link

mrljsh commented Mar 18, 2024

Really?

What is this guide about then? https://biomejs.dev/guides/big-projects/

@ematipico
Copy link
Member

ematipico commented Mar 19, 2024

Really?

What is this guide about then? biomejs.dev/guides/big-projects

The guide explains to have a command in each package, NOT in the root. You can't do both, which is what you're trying to do.

I am not sure how your project is set up, but if each package has its own check command, that guide is for you. But if you need to have a command at the root of the repository AND tweak something for each package, you should use overrides to tweak each package with different rules/configs.

We have some planned tasks to improve LSP workspaces (they aren't package manager workspaces/monorepos).

For package manager workspaces/monorepos, we still need to design few things in order to make things working both in the CLI and LSP.

@synecdokey
Copy link

synecdokey commented Mar 19, 2024

We experience the same issue, with a single biome.jsonc (using default rules for the formatting) at the root, but only occasionally, running the same exact command, in the exact same spot. Something about jsx and/or ternaries doesn't seem fully deterministic.

› pnpm pkg:check:fix
> biome check . --apply
Checked 2112 files in 329ms. No fixes needed.
> pnpm pkg:check:fix
› biome check. --apply
Checked 2112 files in 399ms. Fixed 64 files.
› pnpm pkg:check:fix
> biome check . --apply
Checked 2112 files in 432ms. Fixed 64 files.
› pnpm pkg:check:fix
› biome check . --apply
Checked 2112 files in 2s. No fixes needed.

Code is a jsx ternary every time. Like this:

- {AppWrapper
-   ? <AppWrapper>
-     <App {... product} />
- </AppWrapper>}
- : <AppWrapper {...product} />
+ {AppWrapper ? (
+   <AppWrapper>
+     <App {... product} />
+   </AppWrapper>
+ ) : (
+   <App {... product) />
+ )}

@ematipico
Copy link
Member

Is there any chance someone who experiences the issue can share a minimal reproduction? It suspect the cause of the issue is caused by how we use ignore and include

@ematipico
Copy link
Member

Can anyone check if v1.6.2 fixes the issue?

@bc-m
Copy link
Author

bc-m commented Mar 22, 2024

Can anyone check if v1.6.2 fixes the issue?

Still an issue. I try to create an minimal reproduction for you.

@bc-m
Copy link
Author

bc-m commented Mar 22, 2024

Here you go:

  1. Clone https://github.com/bc-m/biome-issue-2080-minimal-reproduction
  2. Open ./src/Test.spec.tsx
  3. Execute npm run biome:correct-result for expected result for ./src/Test.spec.tsx
  4. Execute npm run biome:wrong-result for non-expected result for ./src/Test.spec.tsx
  5. Repeat step 3 and 4 until your going crazy 🤪

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Project Area: project 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.

6 participants