-
-
Notifications
You must be signed in to change notification settings - Fork 511
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
☂️ Prettier Divergence collection from the Prettier challenge #739
Comments
DivergenceBogus or duplicate modifiers on class properties Input // Multiple accessibility modifiers
class Foo {
private public a = 1;
}
// Declare function with body
declare function foo ( ) { }
// Invalid use of abstract
class Bar {
abstract foo ;
}
// Duplicate Readonly
class Read {
readonly readonly x: number;
} Diff // Multiple accessibility modifiers
class Foo {
- private a = 1;
+ private public a = 1;
}
// Declare function with body
-declare function foo() {};
+declare function foo ( ) { }
// Invalid use of abstract
class Bar {
- abstract foo;
+ abstract foo ;
}
// Duplicate Readonly
class Read {
- readonly x: number;
+ readonly readonly x: number;
} Related Prettier Tests
ReasonPrettier's Babel-based parsing for JS/TS is very loose and allows multiple errors to be ignored. Biome's parser intentionally has stricter rules than Prettier and correctly asserts that the statements in these tests are invalid. Duplicate modifiers for functions are not semantically valid, function declarations are not allowed to have bodies, non-abstract classes cannot have abstract properties, etc. In Prettier, these errors aren't considered parse errors, and the AST is still built "correctly" with the appropriate nodes. When formatting, Prettier then just treats these nodes as normal and formats them accordingly. In Biome, the parsing errors result in For class properties, Prettier's current parsing strategy also uses boolean fields for modifiers, meaning only one of each kind of modifier can ever be present (accessibility modifiers are stored as a single string). When printing, Prettier just looks at the list of booleans and decides which modifiers to print out again. Biome instead keeps a list of modifiers, meaning duplicates are kept around and able to be analyzed (hence the parsing error messages about duplicate modifiers and ordering). When printing out the bogus nodes, this list is still kept intact, and printing out the unformatted text results in those modifiers continuing to exist. There are ways that Biome can address this. One possibility is to try to interpret the Bogus nodes when formatting and construct valid nodes out of them. If a valid node can be built, then it would just format that node like normal, otherwise it prints the bogus text verbatim as it does currently. However, this is messy and introduces a form of parsing logic into the formatter that is really not meaningful. Another option instead is to introduce some form of "syntactically-valid bogus node" into the parser, which accepts these kinds of errors that are purely semantic (duplicate modifiers, abstract properties in non-abstract classes). It would continue to build the nodes like normal (effectively matching the behavior in Prettier), but store them inside of a the new kind of bogus node, including the diagnostics along with it. When formatting, these special bogus nodes would just attempt to format the inner node, and then fallback if there's an error (the existing After a little bit of discussion in Discord, the decision for now is to not worry about this divergence unless it's needed to reach 95% compatibility with Prettier. Playground link |
DivergencePrettier doesn't unquote some object properties that are valid JavaScript identifiers. Input const obj = {
'a': true,
b: true,
"𐊧": true,
} Diff const obj = {
a: true,
b: true,
- "𐊧": true,
+ 𐊧: true,
}; Prettier test ReasonPrettier and Biome unquote object and class properties that are valid JavaScript identifiers. In an ecosystem where ES2015 is now widespread, this seems to be a legacy restriction. A possible workaround would be to introduce a configuration to set the ECMAScript version a project uses. |
DivergencePrettier has an inconsistent behavior for assignment in computed keys. Input a = {
[this.resource = resource]: 1,
} Diff a = {
- [(this.resource = resource)]: 1,
+ [this.resource = resource]: 1,
}; Prettier Test ReasonPrettier and Biome enclose some assignment expressions between parentheses, in particular in conditionals. Prettier has an inconsistent behavior, because it adds parentheses for an assignment in a computed key of an object property and doesn't for a computed key of a class property, as demonstrated by the following example: Input a = {
[x = 0]: 1,
}
class C {
[x = 0] = 1
} Output a = {
[(x = 0)]: 1,
};
class C {
[x = 0] = 1;
} To be consistent, we decide to diverge and omit the parentheses. |
DivergencePrettier accepts an incorrect modifier for the type parameters of an interface. Input interface L<in const T> {} Diff -interface L<const in T> {}
+interface L<in const T> {} Prettier Test ReasonAs mentioned in a previous divergence, Prettier's Babel-based parsing for JS/TS is very loose and allows multiple errors to be ignored. Biome's parser intentionally has stricter rules than Prettier and correctly asserts that the In Prettier, this error isn't considered a parse error, and the AST is still built "correctly" with the appropriate nodes. When formatting, Prettier then just treats these nodes as normal and formats them accordingly. In Biome, the parsing errors result in a Bogus node, which may contain any number of valid nodes, invalid nodes, and/or raw tokens. When formatting, Biome treats bogus nodes as effectively plain text, printing them out verbatim into the resulting code without any formatting, since attempting to format could be incorrect and cause semantic changes. We could adopt the same strategy that discussed in a previous comment to fix the divergence. |
Important This divergence is now fixed DivergencePrettier keeps parentheses around optional chains. PrettierTests Reason
|
Important This divergence is now fixed DivergencePrettier adds a trailing comma to some type parameter lists of arrow functions to distinguish them from JSX elements. Prettier Tests
Reason
In some cases, a trailing comma is required to distinguish an arrow function from a JSX element. We fixed the divergence by making the trailing comma mandatory in these specific cases. |
Removing the parentheses here changes the meaning of the code. The call or member expression becomes part of the optional chain.
Prettier does this only in TSX to disambiguate generics from JSX elements. |
That has been updated, changelog |
You are right. In fact, Biome already keeps the trailing comma. However, Biome seems not able to parse the
Thanks for highlighting this. |
DivergencePrettier adds a trailing comma to type parameters of arrow functions even when it is not required. Input ( <T = unknown>() => {}; Diff - <T = unknown,>() => {};
+ <T = unknown>() => {}; ReasonIn some specific cases, a type parameter list of an arrow function requires a trailing comma to distinguish it from a JSX element. |
Divergence (off-topic, suggested)Input ( const A = {
x: "x",
y: "y",
z: "z",
};
const {
x,
y,
z,
} = A; Diff const A = {
x: "x",
y: "y",
z: "z",
};
- const {x, y, z} = A;
+ const {
+ x,
+ y,
+ z,
+ }; Test https://github.com/biomejs/biome/blob/main/crates/biome_js_formatter/report.md#jsassignmentdestructuringjs ReasonThere was never any good reason given by Prettier why these should be treated differently. In an object literal, if the developer enters a newline then Prettier formats it with all newlines. The same should be done for destructuring. |
Pretty sure the last case depends on the line length of the destructuring statement, past certain length it newlines all the keys. IMO Prettier is in the right there because newlining in all cases feels not so much different from doing |
No, you missed the part where the developer has a choice with object literals, but not with destructuring. Prettier does wrap destructured assignments and object literals when the go past the line wrap limit. In an object literal though, when the developer places a newline their themselves then Prettier puts all props on newlines regardless of the line wrap limit. So what is the reasoning to not do the same thing for destructuring? I'd love to hear a reason why developers should have this option in one spot, but not the other. Thanks. |
@waynebloss This issue isn't the right place for this discussion, but see https://prettier.io/docs/en/rationale#%EF%B8%8F-a-note-on-formatting-reversibility |
This thread is specifically to list out cases where Biome has already intentionally decided to deviate from Prettier's behavior and explain the rationale for that as an easily accessible source of truth for any differences that people might encounter. It's not really intended as a place to point out discrepancies between Biome or Prettier or seek clarification about them. If you'd like to ask why Biome is different than Prettier in a specific case, then opening an issue about it would be the best place to start (or asking in Discord, or a Discussion). If it is then decided to not be remedied by changing Biome, then a solidified, well-established answer will be added to this thread. If you'd like to ask why Prettier decides to format things in a certain way (i.e., Biome does this because Prettier does, but why does Prettier do it), then the Prettier repository would be the appropriate place to ask. In this situation, Biome is just matching Prettier's behavior, so the question should be asked to the Prettier team, or asked in a new issue for Biome to consider deviating. |
Ahhh, thanks @faultyserver that was not clear. |
Thank you everyone, I'm going to close the issue, as it solved its purpose. |
NOTE: This issue thread is for explaining intentional divergences from Prettier. It is not intended as a place to seek clarification about formatting rules or point out discrepancies with Prettier. For those, please open a new issue or discussion. Once a decision has been made, a new entry will be added to this thread explaining the rationale and purpose behind choosing to diverge from Prettier's rules.
This Issue will function as a hub for collecting divergence from Prettier. When you want to comment, please use the following template:
The text was updated successfully, but these errors were encountered: