feat(rome_js_analyze): compute AnalyzerOptions from Workspace
#3381
Conversation
2828017 to
c3590b0
Compare
✅ Deploy Preview for rometools ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
Comparing feat(rome_js_analyze): compute
1 page tested
|
| Chrome Desktop | iPhone, 4G LTE | Motorola Moto G Power, 3G connection |
|---|---|---|
 |
 |
 |
Most significant changes
 |
Value | Budget |
|---|---|---|
| Cumulative Layout Shift Chrome Desktop |
0.237  from 0.006 |
 |
| Total JavaScript Size in Bytes Chrome Desktop |
1.24 MB  from 86.8 KB |
|
| Total JavaScript Size in Bytes iPhone, 4G LTE |
1.24 MB from 86.8 KB |
|
| Total JavaScript Size in Bytes Motorola Moto G Power, 3G connection |
1.24 MB from 86.8 KB |
|
| Number of Requests Chrome Desktop |
39  from 5 |
|
5 other significant changes: Number of Requests on iPhone, 4G LTE, Number of Requests 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
Calibre: Site dashboard | View this PR | Edit settings | View documentation
| } else { | ||
| vec![] | ||
| }; | ||
| to_globals: ToGlobals, |
There was a problem hiding this comment.
My understanding is that globals are a JS specific concept. Shouldn't it be handled outside of the generic to_analyzer_configuration or how do we plan to support other languages?
There was a problem hiding this comment.
It was, but now they can be a CSS thing too :)
Imagine a third party CSS that creates a variable --external-font-size, and it's used inside the user's code:
.some_class {
font-size: var(--external-font-size) * 0.2
}There was a problem hiding this comment.
OK. I think my main concern is how this approach scales with new settings that are common to some languages (globals e.g. aren't supported by JSON). Is our intention to add a closure for every such concept?
Anyway: This is unrelated to your change. Only something that popped up because I was confused about that to_globals call.
There was a problem hiding this comment.
Unfortunately this approach won't scale that well, at least not as much as the same approach we have for FormatOptions -> JsFormatOptions. Initially I wanted to apply the same approach, but I encountered many complex issues that would have required refactors - which I am not yet comfortable with - inside the analyzer itself.
Summary
Closes #3392
I also changed the signature of the function
runto result aResult. This would allow us to track the diagnostics emitted from errors. The tracking of the errors will be done in a separate PR, because it requires a refactor of the current diagnostics of the analzyer, which still use the old diagnostics.Test Plan
Unfortunately I can't test my changes yet. The current CI should work