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

📝 Mixed Tab/Space on ternary #1661

Closed
1 task done
eMerzh opened this issue Jan 25, 2024 · 12 comments · Fixed by #2078
Closed
1 task done

📝 Mixed Tab/Space on ternary #1661

eMerzh opened this issue Jan 25, 2024 · 12 comments · Fixed by #2078
Assignees

Comments

@eMerzh
Copy link
Contributor

eMerzh commented Jan 25, 2024

Environment information

CLI:
  Version:                      1.5.3
  Color support:                true

Platform:
  CPU Architecture:             aarch64
  OS:                           macos

Environment:
  BIOME_LOG_DIR:                unset
  NO_COLOR:                     unset
  TERM:                         "xterm-256color"
  JS_RUNTIME_VERSION:           "v21.5.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": "https://biomejs.dev/schemas/1.5.3/schema.json",
	"formatter": {
		"indentStyle": "tab",
		"lineWidth": 120,
		"ignore": ["**/__generated__/*.graphql.ts"]
	},
	"javascript": {
		"formatter": {
			"semicolons": "asNeeded",
			"arrowParentheses": "asNeeded",
			"quoteStyle": "single",
			"jsxQuoteStyle": "double"
		}
	}
}

Playground link

https://biomejs.dev/playground/?lineWidth=120&quoteStyle=single&semicolons=as-needed&arrowParentheses=as-needed&unsafeParameterDecoratorsEnabled=false&code=ZQB4AHAAbwByAHQAIABjAG8AbgBzAHQAIABTAHQAbwByAHkATgBhAHQAaQB2AGUAQwByAGUAYQB0AGUAIAA9ACAAewAKAAkAbgBhAG0AZQA6ACAAJwBTAHQAbwByAHkATgBhAHQAaQB2AGUAQwByAGUAYQB0AGUAJwAsAAoACQBwAGEAdABoADoAIAAnAG0AbwBiAGkAbABlAC8AZQBkAGkAdABvAHIAJwAsAAoACQBwAHIAZQBwAGEAcgBlAFYAYQByAGkAYQBiAGwAZQBzADoAIAAoAF8AOgAgAHMAdAByAGkAbgBnACwAIAB7ACAAbABvAGMAYQB0AGkAbwBuACAAfQA6ACAAewAgAGwAbwBjAGEAdABpAG8AbgA6ACAAewAgAHEAdQBlAHIAeQA%2FADoAIAB7ACAAcwBjAGgAZQBtAGEASQBkAD8AOgAgAHMAdAByAGkAbgBnACAAfQAgAH0AIAB9ACkAIAA9AD4ACgAJAAkAbABvAGMAYQB0AGkAbwBuAC4AcQB1AGUAcgB5AD8ALgBzAGMAaABlAG0AYQBJAGQACgAJAAkACQA%2FACAAewAKAAkACQAJAAkACQBzAGMAaABlAG0AYQBJAGQAOgAgAGwAbwBjAGEAdABpAG8AbgAuAHEAdQBlAHIAeQAuAHMAYwBoAGUAbQBhAEkAZAAsAAoACQAJAAkACQB9AAoACQAJAAkAOgAgAHsAfQAsAAoACQBnAGUAdABDAG8AbQBwAG8AbgBlAG4AdAA6ACAAKAApACAAPQA%2BACAAaQBtAHAAbwByAHQAKAAvACoAIAB3AGUAYgBwAGEAYwBrAEMAaAB1AG4AawBOAGEAbQBlADoAIAAiAG4AYQB0AGkAdgBlACIAIAAqAC8AIAAnAC4ALwBTAHQAbwByAHkATgBhAHQAaQB2AGUAQwByAGUAYQB0AGUAJwApAC4AdABoAGUAbgAoAG0AbwBkACAAPQA%2BACAAbQBvAGQALgBkAGUAZgBhAHUAbAB0ACkALAAKAH0A

Code of Conduct

  • I agree to follow Biome's Code of Conduct
@eMerzh
Copy link
Contributor Author

eMerzh commented Jan 25, 2024

hum, sorry for the noise, it' doesn't seems to showup in the playground but

export const StoryNativeCreate = {
	name: 'StoryNativeCreate',
	path: 'mobile/editor',
	prepareVariables: (_: any, { location }: { location: { query?: { schemaId?: string } } }) =>
		location.query?.schemaId
			? {
					schemaId: location.query.schemaId,
				}
			: {},
	getComponent: () => import(/* webpackChunkName: "native" */ './StoryNativeCreate').then(mod => mod.default),
}

seems to be reformated with a double space before the closing }

Capture d’écran 2024-01-25 à 10 36 18

@ematipico ematipico closed this as not planned Won't fix, can't repro, duplicate, stale Jan 25, 2024
@ematipico
Copy link
Member

ematipico commented Jan 25, 2024

Ternaries are weird in prettier.

The initial indentation is done using the indentation style configured in the options. That indentation is done until the ternary operator starts. Then the formatter adds two spaces for each level of nested condition .

Try to add multiple nested conditions and you'll see the difference.

Our formatter matches prettier's, so it's consistent.

@eMerzh
Copy link
Contributor Author

eMerzh commented Jan 25, 2024

ok, i believe you ... but if i run prettier & biome on the same code the result is different ...

@ematipico ematipico reopened this Jan 25, 2024
@ematipico
Copy link
Member

Thank you, that wasn't very clear to me.

Can you please provide two playground links, one from biome and one from prettier?

We will take a look

@ematipico
Copy link
Member

It seems that prettier is actually buggy. The indentation of } isn't respected.

Screenshot 2024-01-31 at 11 14 20

@eMerzh
Copy link
Contributor Author

eMerzh commented Jan 31, 2024

i think it's due to the tabwith ..
like if you display the tab chars it makes more sense (ofc it doesn't look good in the playground with incorrect tabwith

Capture d’écran 2024-01-31 à 12 28 25

@ematipico
Copy link
Member

You may be using a different version of prettier. Our playground uses an outdated version of Prettier, but Prettier's Playground uses its latest version.

Still, I don't think it's a bug unless we wait for prettier to fix their issue.

@eMerzh
Copy link
Contributor Author

eMerzh commented Jan 31, 2024

sorry, i really don't want to be pushy, but i think i'm and prettier's playground are on the same version , the latest (3.2.4)

the issue is that the playground displays the tabs as bigger
if you (ultra crappy) change the tab size to 2 artificially, and make them visible (using '.' for example)

like (😬 )
$$('.cm-tab').forEach(e=> {console.log(e.innerHTML ='..')})
you end up with the something similar to 👆

Capture d’écran 2024-01-31 à 12 53 24

and wich , IMHO, seems fine like this (doesn't mix tab and space for prefix indentation)

@si14 si14 mentioned this issue Feb 7, 2024
1 task
github-merge-queue bot pushed a commit to tldraw/tldraw that referenced this issue Feb 7, 2024
Biome as it is now didn't work out for us 😢 

Summary for posterity:

* it IS much, much faster, fast enough to skip any sort of caching
* we couldn't fully replace Prettier just yet. We use Prettier
programmatically to format code in docs, and Biome's JS interface is
officially alpha and [had legacy peer deps
set](biomejs/biome#1756) (which would fail our
CI build as we don't allow installation warnings)
* ternary formatting differs from Prettier, leading to a large diff
biomejs/biome#1661
* import sorting differs from Prettier's
`prettier-plugin-organize-imports`, making the diff even bigger
* the deal breaker is a multi-second delay on saving large files (for us
it's
[Editor.ts](https://github.com/tldraw/tldraw/blob/main/packages/editor/src/lib/editor/Editor.ts))
in VSCode when import sorting is enabled. There is a seemingly relevant
Biome issue where I posted a small summary of our findings:
biomejs/biome#1569 (comment)

Further actions:

* reevaluate in a few months as Biome matures

### Change Type

- [x] `internal` — Any other changes that don't affect the published
package
@ematipico ematipico self-assigned this Mar 13, 2024
@eMerzh
Copy link
Contributor Author

eMerzh commented Mar 13, 2024

this awesome! i'll thanks for your work :)

i'll let you know when this is testable :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants