fix(rome_analyze): move config to a more performant place #3542

xunilrj · 2022-10-31T23:00:06Z

Summary

This PR does two things:

Move the config deserialization to a more performant place;
Allow custom code around the config of rules; In this specific case deserialize the config from a vec, for a more easi configuration schema, to a HashMap, a mor performant data structure.

Test Plan

cargo test

netlify · 2022-10-31T23:00:11Z

✅ Deploy Preview for docs-rometools canceled.

Name	Link
🔨 Latest commit	`e09a348`
🔍 Latest deploy log	https://app.netlify.com/sites/docs-rometools/deploys/63852bc041cafb00075c5433

github-actions · 2022-10-31T23:07:00Z

Playground for commit 8eb0176

crates/rome_analyze/src/context.rs

xunilrj · 2022-11-01T15:10:59Z

!bench_analyzer

github-actions · 2022-11-01T15:23:13Z

Analyzer Benchmark Results

group                     main                                   pr
-----                     ----                                   --
analyzer/css.js           1.00      2.9±0.08ms     4.0 MB/sec    1.00      2.9±0.04ms     4.0 MB/sec
analyzer/index.js         1.00      8.5±0.27ms     3.8 MB/sec    1.01      8.5±0.28ms     3.8 MB/sec
analyzer/lint.ts          1.00      3.7±0.01ms    11.2 MB/sec    1.04      3.8±0.07ms    10.9 MB/sec
analyzer/parser.ts        1.00      9.6±0.12ms     5.1 MB/sec    1.01      9.7±0.08ms     5.0 MB/sec
analyzer/router.ts        1.00      7.0±0.03ms     8.8 MB/sec    1.02      7.1±0.04ms     8.6 MB/sec
analyzer/statement.ts     1.00     10.2±0.07ms     3.5 MB/sec    1.03     10.5±0.08ms     3.4 MB/sec
analyzer/typescript.ts    1.00     17.6±0.78ms     3.1 MB/sec    1.01     17.8±0.51ms     3.1 MB/sec

crates/rome_analyze/src/services.rs

crates/rome_js_analyze/src/lib.rs

ematipico · 2022-11-03T13:57:25Z

crates/rome_js_analyze/tests/spec_tests.rs

-    let options = AnalyzerOptions::default();
+    let mut options = AnalyzerOptions::default();
+
+    if let Ok(value) = std::env::var("ROME_TEST_RULE_OPTIONS") {


Where is ROME_TEST_RULE_OPTIONS coming from? Do we have it somewhere? If so, we should document it

I am actually changing the implementation to allow the rule to be configured inside each test file. Makes more sense.

calibre-analytics · 2022-11-04T11:19:20Z

Comparing fix(rome_analyze): move config to a more performant place Snapshot #12 to median since last deploy of rome.tools.

	LCP^?	CLS^?	TBT^?
Overall ^{Median across all pages and test profiles}	2.12s ^from 281ms	0.0 ^no change	157ms ^no change
Chrome Desktop ^{Chrome Desktop • Cable}	2.12s ^from 281ms	0.0 ^no change	300ms ^from 4ms
iPhone, 4G LTE ^{iPhone 12 • 4G LTE}	1.06s ^from 246ms	0.0 ^no change	23ms ^no change
Motorola Moto G Power, 3G connection ^{Motorola Moto G Power • Regular 3G}	16.5s ^from 1.06s	0.0 ^no change	157ms ^no change

1 page tested

Home

Browser previews

Chrome Desktop	iPhone, 4G LTE	Motorola Moto G Power, 3G connection

Most significant changes

	Value	Budget
Total Blocking Time Chrome Desktop	300ms ^from 4ms
JS Parse & Compile Motorola Moto G Power, 3G connection	1.76s ^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, First Contentful Paint on Motorola Moto G Power, 3G connection, Largest 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, Number of Requests on Motorola Moto G Power, 3G connection, Number of Requests on Chrome Desktop, Number of Requests on iPhone, 4G LTE, Largest Contentful Paint on Chrome Desktop, First Contentful Paint on Chrome Desktop, 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}

crates/rome_analyze/src/context.rs

github-actions · 2022-11-25T12:03:51Z

This PR is stale because it has been open 14 days with no activity.

ematipico

Code looks good, but we should update the contribution guidelines

ematipico · 2022-11-28T11:08:15Z

crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/malformedOptions.js

@@ -0,0 +1 @@
+/* Options: {"hook": [["useMyEffect", 0, 1]]} */


What about a .json file instead? We use the same approach in the formatter

Also, would you mind updating the contributions guidelines to explain how to add options to rules?

Done. Now we can configure each test with a special file.

Addressed

xunilrj temporarily deployed to netlify-playground October 31, 2022 23:00 Inactive

xunilrj temporarily deployed to netlify-playground November 1, 2022 10:39 Inactive

xunilrj temporarily deployed to netlify-playground November 1, 2022 11:45 Inactive

rome deleted a comment from github-actions bot Nov 1, 2022

MichaReiser reviewed Nov 1, 2022

View reviewed changes

crates/rome_analyze/src/context.rs Show resolved Hide resolved

xunilrj temporarily deployed to netlify-playground November 1, 2022 15:09 Inactive

rome deleted a comment from github-actions bot Nov 1, 2022

xunilrj temporarily deployed to netlify-playground November 3, 2022 11:44 Inactive

xunilrj marked this pull request as ready for review November 3, 2022 12:12

xunilrj requested a review from leops as a code owner November 3, 2022 12:12

leops reviewed Nov 3, 2022

View reviewed changes

crates/rome_analyze/src/services.rs Outdated Show resolved Hide resolved

ematipico reviewed Nov 3, 2022

View reviewed changes

xunilrj temporarily deployed to netlify-playground November 4, 2022 11:17 Inactive

xunilrj temporarily deployed to netlify-playground November 4, 2022 12:35 Inactive

leops reviewed Nov 4, 2022

View reviewed changes

crates/rome_analyze/src/context.rs Outdated Show resolved Hide resolved

xunilrj mentioned this pull request Nov 8, 2022

☂️ Lint rules from other sources #3358

Closed

11 tasks

MichaReiser added this to the 11.0.0 milestone Nov 10, 2022

ematipico mentioned this pull request Nov 15, 2022

☂️ Linter 11.0.0 #3739

Closed

29 tasks

github-actions bot added the S-Stale label Nov 25, 2022

xunilrj force-pushed the feature/improv-rule-options branch from c963424 to 31a0608 Compare November 26, 2022 14:46

xunilrj requested a review from a team as a code owner November 26, 2022 14:46

xunilrj force-pushed the feature/improv-rule-options branch from e1b39df to 4e65a85 Compare November 28, 2022 10:32

ematipico previously requested changes Nov 28, 2022

View reviewed changes

github-actions bot removed the S-Stale label Nov 28, 2022

xunilrj added 15 commits November 28, 2022 18:43

move config to a more performant place

448328d

codegen and fmt and clippy issues

8471eb7

fix documentation

7a8fe87

trying to optimize rule option retrieve

9e89859

clippy and fmt issues

1042773

rule options wrapper for the service bag

57e0713

fmt and clippy issues

03bafa7

option deserialization generating analyzer signals

266d874

clippy and fmt issues

2ff57f5

rebase

7cc3015

clippy and fmt issues

be8dc00

clippy and fmt issues

c2fca5d

codegen issues

7584cdb

lint rule options using files and contributing rules

26cd098

codegen issues

b301dba

xunilrj force-pushed the feature/improv-rule-options branch from f437772 to b301dba Compare November 28, 2022 18:51

fix rebase issues

e09a348

xunilrj merged commit 00f5dfe into main Nov 28, 2022

xunilrj deleted the feature/improv-rule-options branch November 28, 2022 22:00

notmd mentioned this pull request Nov 29, 2022

chore(rome_js_analyze): remove dbg macro #3894

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(rome_analyze): move config to a more performant place #3542

fix(rome_analyze): move config to a more performant place #3542

xunilrj commented Oct 31, 2022

netlify bot commented Oct 31, 2022 •

edited

Loading

github-actions bot commented Oct 31, 2022 •

edited

Loading

xunilrj commented Nov 1, 2022

github-actions bot commented Nov 1, 2022

ematipico Nov 3, 2022

xunilrj Nov 26, 2022

calibre-analytics bot commented Nov 4, 2022 •

edited

Loading

Browser previews

Most significant changes

github-actions bot commented Nov 25, 2022

ematipico left a comment

ematipico Nov 28, 2022

xunilrj Nov 28, 2022

		@@ -0,0 +1 @@
		/* Options: {"hook": [["useMyEffect", 0, 1]]} */

fix(rome_analyze): move config to a more performant place #3542

fix(rome_analyze): move config to a more performant place #3542

Conversation

xunilrj commented Oct 31, 2022

Summary

Test Plan

netlify bot commented Oct 31, 2022 • edited Loading

✅ Deploy Preview for docs-rometools canceled.

github-actions bot commented Oct 31, 2022 • edited Loading

xunilrj commented Nov 1, 2022

github-actions bot commented Nov 1, 2022

Analyzer Benchmark Results

ematipico Nov 3, 2022

Choose a reason for hiding this comment

xunilrj Nov 26, 2022

Choose a reason for hiding this comment

calibre-analytics bot commented Nov 4, 2022 • edited Loading

1 page tested

Browser previews

Most significant changes

github-actions bot commented Nov 25, 2022

ematipico left a comment

Choose a reason for hiding this comment

ematipico Nov 28, 2022

Choose a reason for hiding this comment

xunilrj Nov 28, 2022

Choose a reason for hiding this comment

netlify bot commented Oct 31, 2022 •

edited

Loading

github-actions bot commented Oct 31, 2022 •

edited

Loading

calibre-analytics bot commented Nov 4, 2022 •

edited

Loading