feat(rome_json_formatter): Enable JSON formatting for debug builds #3859
Conversation
3e033ef to
6d32d2e
Compare
✅ Deploy Preview for docs-rometools canceled.
|
6d32d2e to
54950eb
Compare
| @@ -372,44 +375,57 @@ impl Workspace for WorkspaceServer { | |||
| &self, | |||
| params: PullDiagnosticsParams, | |||
| ) -> Result<PullDiagnosticsResult, RomeError> { | |||
| let capabilities = self.get_capabilities(¶ms.path); | |||
There was a problem hiding this comment.
I had to change the pull_diagnostics implementation to return the parse diagnostics if linting isn't supported instead of emitting an "unsupported" error.
Not sure if the way I did it is the "best".
There was a problem hiding this comment.
You'd still want to use the lint function, even when the actual linter is disabled. When the linter is disabled, you'd need to create the AnalysisFilter with categories: RuleCategories::SYNTAX instead.
This is necessary because we could have analyzers that work at syntax level. An example is noSuperWithoutExtends
There was a problem hiding this comment.
How would I call into the lint function if it is None? It isn't testing if the linter is disabled, it is testing if the language supports linting.
My understanding is that self.get_capabilities returns the capabilities of the language, which is always Some for languages that support linting and only is None for JSON unrelated if the linter is enabled or not.
There was a problem hiding this comment.
Unfortunately there's a slight mismatch between "capabilities" and "features": what the lint capability actually represents is whether we have a rome_<lang>_analyze crate for this language (this includes syntax rules, lint rules and assist rules), and not just whether we support linting for this language
035c750 to
9ab6592
Compare
|
Comparing feat(rome_json_formatter): Enable JSON formatting for debug builds Snapshot #3 to median since last deploy of rome.tools.
1 page tested
|
| Chrome Desktop | iPhone, 4G LTE | Motorola Moto G Power, 3G connection |
|---|---|---|
 |
 |
 |
Most significant changes
 |
Value | Budget |
|---|---|---|
| Total Blocking Time Chrome Desktop |
613ms  from 4ms |
 |
| JS Parse & Compile Motorola Moto G Power, 3G connection |
2.18s  from 25ms |
|
| Total JavaScript Size in Bytes Chrome Desktop |
5.37 MB  from 86.8 KB |
|
| Total JavaScript Size in Bytes iPhone, 4G LTE |
5.37 MB from 86.8 KB |
|
| Total JavaScript Size in Bytes Motorola Moto G Power, 3G connection |
5.37 MB from 86.8 KB |
|
27 other significant changes: JS Parse & Compile on iPhone, 4G LTE, JS Parse & Compile on Chrome Desktop, Largest Contentful Paint on Motorola Moto G Power, 3G connection, First Contentful Paint on Motorola Moto G Power, 3G connection, Total CSS Size in Bytes on Chrome Desktop, Total CSS Size in Bytes on iPhone, 4G LTE, Total CSS Size in Bytes on Motorola Moto G Power, 3G connection, Time to Interactive on Motorola Moto G Power, 3G connection, Total Page Size in Bytes on Chrome Desktop, Total Page Size in Bytes on iPhone, 4G LTE, Total Page Size in Bytes on Motorola Moto G Power, 3G connection, Time to Interactive on Chrome Desktop, Largest Contentful Paint on Chrome Desktop, First Contentful Paint on Chrome Desktop, Number of Requests on Chrome Desktop, Number of Requests on Motorola Moto G Power, 3G connection, Number of Requests on iPhone, 4G LTE, Speed Index on Motorola Moto G Power, 3G connection, Time to Interactive on iPhone, 4G LTE, First Contentful Paint on iPhone, 4G LTE, Largest Contentful Paint on iPhone, 4G LTE, Speed Index on Chrome Desktop, Total HTML Size in Bytes on Chrome Desktop, Total HTML Size in Bytes on iPhone, 4G LTE, Total HTML Size in Bytes on Motorola Moto G Power, 3G connection, Lighthouse Performance Score on Motorola Moto G Power, 3G connection, Lighthouse Performance Score on Chrome Desktop
Calibre: Site dashboard | View this PR | Edit settings | View documentation
|
I created a |
Summary
This PR enables the formatting capability for JSON for debug builds.
Enabling the JSON formatter for debug builds is useful to integrate it into the prettier test suite (that uses
App) and to test rome locally.Why not
rome_flags.is_unstable: We may want to use the unstable flag shortly when at least some formatting is implemented. However, the formatter isn't doing any formatting yet.Test Plan
rome format rome.jsonin the root directory: Rome reports that the file has been formattedrome check rome.json: Rome emits a diagnostic that linting isn't supportedrome ci rome.json: Rome reports that it checked one file