Shared Formatter infrastructure #2525
Replies: 4 comments 3 replies
-
Lol, I guess I just fell for the same trap again. Implementing |
Beta Was this translation helpful? Give feedback.
-
Related Work |
Beta Was this translation helpful? Give feedback.
-
Ah yes, the orphan rule. Probably the top gripe I have with Rust. I know this might be a ugly choice from a call-site perspective, but could we forgo traits entirely and just have a bunch of formatter methods? Like With |
Beta Was this translation helpful? Give feedback.
-
I think that every logic that we have that we try to share should not be shared. I've come to the conclusion that the main goal here is to try to create a shared infrastructure where any data structure ( Now, this has the big downside that most of the code we've created so far can't be reused for other languages (maybe the bare minimum about reusing text slices and comments), but from one point of view it kind of makes sense. Trivias might work differently in another language, lists too, etc. Option 3 seems to be the best solution, but I'm not very fond of it. |
Beta Was this translation helpful? Give feedback.
-
I've been extracting the language-agnostic formatter infrastructure into
rome-formatter
until I hit a problem that Rust (for good reasons), doesn't allow to implement of a trait if both, the trait and the type are foreign to the current crate.I hit this problem when I moved the
Format
trait torome-formatter
because:rome-formatter
defines theFormat
traitrome-js-syntax
defines theAstNode*
typesThus, neither
Format
nor theAstNode
types are part ofrome-js-formatter
.I'll go through the different options that I considered and explain why I decided for the last. I'll start with the options that I ruled out. My proposed option comes last.
Option 1
One option is to extract the shared formatter logic to
rome-formatter
and implement the formatters insiderome-js-syntax
. This lifts Rust's constraint becauserome-js-syntax
defines theAstNode
s.The reason I ruled out this option is that it makes
rome-js-syntax
and all its dependencies depend on the formatting logic, including the parser. This will most likely result in cyclic dependencies and a significant slowdown in development speed because of the increased compilation times (rome-js-formatter and rome-js-syntax already the slowest crates when it comes to compilation time and combining the two will only make it worse).Option 2
The second option is to forgo with the goal to have different
rome-<language>-formatter
crates for every language. This lifts Rust's constraint becauserome-formatter
defines theFormat
trait.This option probably works well for now where we only have very few languages but I don't see it scale to multiple languages. Again,
rome-js-formatter
is one of the slowest crate when it comes to compilation time because it contains a lot of generated code. Adding more languages will make this worse, not just by the absolute time it takes to compile but also because recompilation becomes necessary if:It also negatively impacts downstream dependencies because it becomes impossible to express that a crate only requires JS formatting, but doesn't depend on CSS formatting. That means, every time
rome_formatter
gets recompiled, all downstream dependencies must so as well. This will significantly slow down compilation time and our iteration speed.Option 3
Since moving the formatting to the
Format
trait's crate or moving the formatting to the syntax crates are valid options, the only option to lift Rust's constraint is by introducing newtype wrappers and implementingFormat
for the new-type wrapper.The way I think about this is the same as the
Display
struct forPath
(the reasons why this struct is necessary differs for Path but the idea is the same).The core idea is that each node gets a
format()
method that returns&Format<Node>
. TheFormat<Node>
struct is a new-type wrapper that wraps the node and implementsFormat
.**
rome_formatter
*`rome_js_formatter
The new-type wrappers solve Rust's constraint because the wrapper types are defined in the
rome-<language>-formatter
traits, and thus, are local to the crate.The main downsides of this approach are:
AsFormat
traitHowever, in my opinion the downsides are outweighed by the following upsides
What are your thoughts?
Future
I hope to soon replace
format_elements
with a macro that instead behaves more likeformat_args![formatter, node1, node2]
. The motivation of doing this is that it's currently a bit of a hazzle because there are two formatting concepts:Format
Something that can be formattedFormatElement
: Some formatted outputand we often want to combine the two. Only having one concept (
Format
) and treatingFormatElement
as pure output of that simplifies the combination of differentFormat
able types.Beta Was this translation helpful? Give feedback.
All reactions