Deploying with    Cloudflare Pages

View logs

Comments from my initial reading:

• Most of the methods on the Rome class should be async, since the backend Rust code may run in a different Worker thread / OS process entirely
• It may be preferable to provide the path of files passed to formatContent or lintContent in order to display it in the emitted diagnostics. This could replace the FileType argument as that would let us use the same file path to file type matching logic as the rest of the toolchain instead of relying on users of the API to re-implement it (drawback: this will require users to "guess" a fake file path that matches the type of the content they're trying to format / lint when working on virtual or generated files)

Comments from my initial reading:

* Most of the methods on the `Rome` class should be async, since the backend Rust code may run in a different Worker thread / OS process entirely

I left that out on purpose because I didn't know the specifics of the architecture. Is it something that you can confirm now or is it too soon? For example esbuild provides synchronous and asynchronous APIs.

* It may be preferable to provide the path of files passed to `formatContent` or `lintContent` in order to display it in the emitted diagnostics. This could replace the `FileType` argument as that would let us use the same file path to file type matching logic as the rest of the toolchain instead of relying on users of the API to re-implement it (drawback: this will require users to "guess" a fake file path that matches the type of the content they're trying to format / lint when working on virtual or generated files)

Yeah, that's true, although providing a path for some content might seem less intuitive, especially for less experienced developers. Also, if we compare this decision against other tools (prettier or eslint), we need to be very good to explain this option that other tools might not have.

How do you envision that caching works for the content methods. Is there no caching, or can users provide an optional explicit id/cache key (which could be a virtual path: virtual://a/b/c.js)

thank you @ematipico for this awesome write-up.

I very much like this API, particularly splitting it into ..Files, ..Content methods.

I added a few inline questions with clarification questions. Two overarching that I have at the moment are:

* Exposing `lintFiles` and `lintDirectories` requires that Rome is limited to run in environments where it's possible to read files. Is Rome's API different in environment that doesn't support IO (e.g. browser, if we're ignoring virtual filesystems)

These APIs are mostly for Node.js, but I can see the future problem.

We have options here:

• release a package/library just for browsers (like esbuild does), which relies exclusively on WebAssembly and exclude those APIs from the library;
• rely on MemoryFileSystem to create virtual files; document that these APIs, on a browser, will work on a virtual file system, no physical files;

Note: I don't know if both options (especially the second) are doable

* It may be interesting to add a few example use cases for the different API's and how they would be used in that case. For example, how would a Jest pretty-printer integration look like? Are there any other immediate use cases that you are aware of that we could use to verify our API, by providing a few sample implementations?

I am open to proposal, because I have no idea how to make a jest pretty-printer. An idea might be a plugin for webpack (like eslint-webpack-plugin, to catch lint errors during compilation, but I don't know how to make a plugin.

I am open to proposal, because I have no idea how to make a jest pretty-printer. An idea might be a plugin for webpack (like eslint-webpack-plugin, to catch lint errors during compilation, but I don't know how to make a plugin.

You may need to spend some time familiarizing yourself. Jest uses pretty-format under the hood. The documentation has a section on how to customize formatting in jest as well as on how to write a plugin

The main challenge that I see is that many such plugin architectures don't support explicit lifetimes (beyond what a GC provides)

EDIT: It seems that Jest only uses Prettier for inline snapshots and I don't think that is configurable except by providing a node module that mimics prettier's API

Edit2: Other use cases

• ESLint plugin
• Github Action
• Pretty Quick
• LintStaged

I spent a little more time thinking about the API and have some more thoughts that may need additional exploration but can be deferred to later or we can decide that we don't want to pursue the idea.

Files, File,Virtual

One thing I noticed about the API is that many functionality provide methods to work on a set of files, a single file, or a virtual file.

rome.formatFiles(["./path"])
rome.formatFile("path"...)
rome.formatContent("content", ...)

An alternative to this could be to have objects that wrap a set of "files":

let files = rome.files(["./path"]);
let file = rome.file("./path");
let virtual = rome.virtual("content", { content-id });

You can then do:

files.check();
file.check();
content.check();

or if we prefer

rome.check(files)
rome.check(file)
rome.check(content)

I haven't thought through the implications of this but thought it interesting.

Rome lifetime

One of the main unanswered questions of this proposal is how to manage Romes lifetime, especially considering the different execution modes:

• Spawn a process for every operation
• Spawn / connect to a deamon
• Create a WASM module

The questions I have are:

• How does a client specify which execution mode they want?
• What does shutdown do? Does it disconnect from a deamon or shuts it down?
• Does the API need equivalents to the CLI's start / stop command and if so, what do they return?

I spent a little more time thinking about the API and have some more thoughts that may need additional exploration but can be deferred to later or we can decide that we don't want to pursue the idea.

Files, File,Virtual

One thing I noticed about the API is that many functionality provide methods to work on a set of files, a single file, or a virtual file.

rome.formatFiles(["./path"])
rome.formatFile("path"...)
rome.formatContent("content", ...)

An alternative to this could be to have objects that wrap a set of "files":

let files = rome.files(["./path"]);
let file = rome.file("./path");
let virtual = rome.virtual("content", { content-id });

You can then do:

files.check();
file.check();
content.check();

or if we prefer

rome.check(files)
rome.check(file)
rome.check(content)

I haven't thought through the implications of this but thought it interesting.

Rome lifetime

One of the main unanswered questions of this proposal is how to manage Romes lifetime, especially considering the different execution modes:

* Spawn a process for every operation

* Spawn / connect to a deamon

* Create a WASM module

The questions I have are:

* How does a client specify which execution mode they want?

I don't think a client should be aware of anything that regards the communication with Rust Workspace. I think that exposing this information would make the APIs more complex, especially for beginners. Our internal architecture should stay internal.

We should decide which kind of communication we should have based on the API. For example, initially we decide that the API .formatContent would use the WASM. After sometime we discovered something and communicating via process is more efficient. We then change it without breaking the official API.

* What does `shutdown` do? Does it disconnect from a deamon or shuts it down?

Not part of this RFC, we can follow up here. I don't have an answer now.

* Does the API need equivalents to the CLI's `start` / `stop` command and if so, what do they return?

Not part of this RFC, we can follow up here. Probably we will reimplement the same logic we had in Rome classic.

We should decide which kind of communication we should have based on the API. For example, initially we decide that the API .formatContent would use the WASM. After sometime we discovered something and communicating via process is more efficient. We then change it without breaking the official API.

I agree that we should for good defaults but I do believe that it should be possible for users to change the default. For example, it may be the most performant to spawn a new CLI for each formatContent call by default, except if these calls are done inside of a loop, in which case it will be more performant to spawn the process only once.

This is why I believe we should expose some sort of control to the user and this may influence how our API looks like significantly which is why I think it's worth to explore the idea at least a bit.

We should decide which kind of communication we should have based on the API. For example, initially we decide that the API .formatContent would use the WASM. After sometime we discovered something and communicating via process is more efficient. We then change it without breaking the official API.

I agree that we should for good defaults but I do believe that it should be possible for users to change the default. For example, it may be the most performant to spawn a new CLI for each formatContent call by default, except if these calls are done inside of a loop, in which case it will be more performant to spawn the process only once.

This is why I believe we should expose some sort of control to the user and this may influence how our API looks like significantly which is why I think it's worth to explore the idea at least a bit.

The only non-intrusive and friendly way I see using these suggestions is by introducing a different sets o APIs, different from the ones proposed in the RFC. An example:

import { workflow } from "rome";

await workflow.withProcess(async romeInstance => { 
	romeInstance.formatContent("");
}).run()

await workflow.withDaemon(async romeInstance => { 
	romeInstance.formatContent("");
}).run()

await workflow.withWasm(async romeInstance => { 
	romeInstance.formatContent("");
}).run()

We can explore this proposal in another RFC.

Using closures has the downside that this isn't working well in situations where an API exposes start and shutdown methods. For example, a webpack loader exposes a function that gets called for every file and ideally, the rome instance would be re-used across files.

An alternative could be

let rome = await Rome.default(); // Creates a new instance using the default scheduling
let rome = await Rome.start(); // Creates a new deamon or connects to an existing one

Or @leops proposed an API where it's possible to pass an optional RomeDeamon API to the format* methods.

let deamon = await RomeDeamon.start()
rome.formatContent(..., deamon)

...
deamon.disconnect() 
// or 
deamon.stop()

Or the deamon could be passed in the constructor?

let deamon = await RomeDeamon.start();

let rome = new Rome(deamon);

One thing we may need to consider with async constructor methods is that not all plugin APIs allow for calling async methods.

RFC updated:

• removed fileType in favour of filePath;
• .parseContent only is exposed, but a .parseFile can be implemented if we want;

Not yet confirmed but evaluated:

• APIs can all be async;
• possibility to choose a back-end when creating a Rome instance;

To evaluate and implement (they require changes in the Rust code):

• resolve the configuration from a path that is not the working directory (the path from where the command is run)