[labs/analyzer] Add support for analyzing function declarations by kevinpschaaf · Pull Request #3655 · lit/lit

kevinpschaaf · 2023-02-07T19:41:56Z

No description provided.

changeset-bot · 2023-02-07T19:41:59Z

🦋 Changeset detected

Latest commit: 4ef6c0a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages

Name	Type
@lit-labs/analyzer	Minor
@lit-labs/cli	Patch
@lit-labs/gen-manifest	Patch
@lit-labs/gen-utils	Patch
@lit-labs/gen-wrapper-angular	Patch
@lit-labs/gen-wrapper-react	Patch
@lit-labs/gen-wrapper-vue	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2023-02-07T19:42:20Z

📊 Tachometer Benchmark Results

Summary

nop-update

lit-html-kitchen-sink: unsure 🔍 -10% - +8% (-2.54ms - +2.00ms)
^{this-change vs tip-of-tree}

render

lit-element-list: 74.56ms - 79.03ms
lit-html-kitchen-sink: unsure 🔍 -2% - +6% (-0.45ms - +1.69ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -6% - +3% (-0.69ms - +0.35ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -2% - +2% (-1.30ms - +1.07ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -1% - +3% (-0.71ms - +1.59ms)
^{this-change vs tip-of-tree}

update

lit-element-list: 682.40ms - 693.35ms
lit-html-kitchen-sink: unsure 🔍 -3% - +5% (-2.12ms - +3.84ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -2% - +3% (-4.30ms - +6.87ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -0% - +1% (-0.50ms - +0.81ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -0% - +1% (-2.87ms - +3.48ms)
^{this-change vs tip-of-tree}

update-reflect

lit-element-list: 684.45ms - 691.70ms
reactive-element-list: unsure 🔍 -1% - +0% (-4.23ms - +2.27ms)
^{this-change vs tip-of-tree}

Results

lit-element-list

Browser: chrome-headless 109.0.5414.119
Sample size: 50
Built by: Benchmarks #4412
Commit: 136782a

render

Version	Avg time	vs
	74.56ms - 79.03ms	-

update

Version	Avg time	vs
	682.40ms - 693.35ms	-

update-reflect

Version	Avg time	vs
	684.45ms - 691.70ms	-

lit-html-kitchen-sink

Browser: chrome-headless 109.0.5414.119
Sample size: 30
Built by: Benchmarks #4412
Commit: 136782a

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	29.83ms - 31.56ms	-	unsure 🔍 -2% - +6% -0.45ms - +1.69ms	unsure 🔍 -2% - +6% -0.65ms - +1.93ms
tip-of-tree tip-of-tree	29.44ms - 30.70ms	unsure 🔍 -5% - +1% -1.69ms - +0.45ms	-	unsure 🔍 -4% - +4% -1.12ms - +1.16ms
previous-release previous-release	29.10ms - 31.01ms	unsure 🔍 -6% - +2% -1.93ms - +0.65ms	unsure 🔍 -4% - +4% -1.16ms - +1.12ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	74.99ms - 78.95ms	-	unsure 🔍 -3% - +5% -2.12ms - +3.84ms	unsure 🔍 -7% - +0% -5.39ms - +0.39ms
tip-of-tree tip-of-tree	73.88ms - 78.34ms	unsure 🔍 -5% - +3% -3.84ms - +2.12ms	-	faster ✔ 0% - 8% 0.29ms - 6.43ms
previous-release previous-release	77.36ms - 81.58ms	unsure 🔍 -1% - +7% -0.39ms - +5.39ms	slower ❌ 0% - 9% 0.29ms - 6.43ms	-

nop-update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	23.33ms - 26.56ms	-	unsure 🔍 -10% - +8% -2.54ms - +2.00ms	unsure 🔍 -6% - +14% -1.42ms - +3.38ms
tip-of-tree tip-of-tree	23.62ms - 26.81ms	unsure 🔍 -8% - +10% -2.00ms - +2.54ms	-	unsure 🔍 -5% - +15% -1.14ms - +3.64ms
previous-release previous-release	22.19ms - 25.74ms	unsure 🔍 -13% - +6% -3.38ms - +1.42ms	unsure 🔍 -14% - +4% -3.64ms - +1.14ms	-

lit-html-repeat

Browser: chrome-headless 109.0.5414.119
Sample size: 30
Built by: Benchmarks #4412
Commit: 136782a

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	11.44ms - 12.16ms	-	unsure 🔍 -6% - +3% -0.69ms - +0.35ms	unsure 🔍 -4% - +4% -0.51ms - +0.52ms
tip-of-tree tip-of-tree	11.59ms - 12.35ms	unsure 🔍 -3% - +6% -0.35ms - +0.69ms	-	unsure 🔍 -3% - +6% -0.35ms - +0.71ms
previous-release previous-release	11.43ms - 12.17ms	unsure 🔍 -4% - +4% -0.52ms - +0.51ms	unsure 🔍 -6% - +3% -0.71ms - +0.35ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	263.65ms - 272.15ms	-	unsure 🔍 -2% - +3% -4.30ms - +6.87ms	unsure 🔍 -2% - +2% -6.46ms - +5.20ms
tip-of-tree tip-of-tree	262.99ms - 270.24ms	unsure 🔍 -3% - +2% -6.87ms - +4.30ms	-	unsure 🔍 -3% - +1% -7.31ms - +3.48ms
previous-release previous-release	264.53ms - 272.53ms	unsure 🔍 -2% - +2% -5.20ms - +6.46ms	unsure 🔍 -1% - +3% -3.48ms - +7.31ms	-

lit-html-template-heavy

Browser: chrome-headless 109.0.5414.119
Sample size: 50
Built by: Benchmarks #4412
Commit: 136782a

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	51.51ms - 53.16ms	-	unsure 🔍 -2% - +2% -1.30ms - +1.07ms	unsure 🔍 -1% - +3% -0.71ms - +1.56ms
tip-of-tree tip-of-tree	51.60ms - 53.31ms	unsure 🔍 -2% - +2% -1.07ms - +1.30ms	-	unsure 🔍 -1% - +3% -0.62ms - +1.70ms
previous-release previous-release	51.13ms - 52.70ms	unsure 🔍 -3% - +1% -1.56ms - +0.71ms	unsure 🔍 -3% - +1% -1.70ms - +0.62ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	104.26ms - 105.26ms	-	unsure 🔍 -0% - +1% -0.50ms - +0.81ms	unsure 🔍 -0% - +1% -0.18ms - +1.11ms
tip-of-tree tip-of-tree	104.18ms - 105.03ms	unsure 🔍 -1% - +0% -0.81ms - +0.50ms	-	unsure 🔍 -0% - +1% -0.28ms - +0.90ms
previous-release previous-release	103.88ms - 104.71ms	unsure 🔍 -1% - +0% -1.11ms - +0.18ms	unsure 🔍 -1% - +0% -0.90ms - +0.28ms	-

reactive-element-list

Browser: chrome-headless 109.0.5414.119
Sample size: 50
Built by: Benchmarks #4412
Commit: 136782a

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	49.08ms - 50.91ms	-	unsure 🔍 -1% - +3% -0.71ms - +1.59ms	unsure 🔍 -2% - +3% -0.79ms - +1.66ms
tip-of-tree tip-of-tree	48.86ms - 50.26ms	unsure 🔍 -3% - +1% -1.59ms - +0.71ms	-	unsure 🔍 -2% - +2% -1.08ms - +1.08ms
previous-release previous-release	48.74ms - 50.38ms	unsure 🔍 -3% - +2% -1.66ms - +0.79ms	unsure 🔍 -2% - +2% -1.08ms - +1.08ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	687.19ms - 691.37ms	-	unsure 🔍 -0% - +1% -2.87ms - +3.48ms	unsure 🔍 -0% - +1% -3.01ms - +3.67ms
tip-of-tree tip-of-tree	686.59ms - 691.37ms	unsure 🔍 -1% - +0% -3.48ms - +2.87ms	-	unsure 🔍 -1% - +1% -3.50ms - +3.56ms
previous-release previous-release	686.35ms - 691.55ms	unsure 🔍 -1% - +0% -3.67ms - +3.01ms	unsure 🔍 -1% - +1% -3.56ms - +3.50ms	-

update-reflect

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	717.93ms - 722.25ms	-	unsure 🔍 -1% - +0% -4.23ms - +2.27ms	unsure 🔍 -1% - +0% -4.42ms - +2.38ms
tip-of-tree tip-of-tree	718.65ms - 723.50ms	unsure 🔍 -0% - +1% -2.27ms - +4.23ms	-	unsure 🔍 -1% - +0% -3.61ms - +3.53ms
previous-release previous-release	718.49ms - 723.73ms	unsure 🔍 -0% - +1% -2.38ms - +4.42ms	unsure 🔍 -0% - +1% -3.53ms - +3.61ms	-

_{tachometer-reporter-action v2 for Benchmarks}

packages/labs/analyzer/src/lib/javascript/functions.ts

bicknellr · 2023-02-07T23:49:14Z

packages/labs/analyzer/src/lib/javascript/functions.ts

+  };
+};
+
+export const getFunctionDeclaration = (


Suggested change

export const getFunctionDeclaration = (

const getFunctionDeclaration = (

bicknellr · 2023-02-08T00:19:40Z

packages/labs/analyzer/src/lib/javascript/functions.ts

+  declaration: ts.FunctionDeclaration,
+  analyzer: AnalyzerInterface
+): DeclarationInfo => {
+  const name = getFunctionDeclarationName(declaration);


Is the other stuff in getFunctionLikeInfo() too expensive to call here? This feels like something that should be rolled into it.

getFunctionLikeInfo() is called here and in ClassMethod

bicknellr · 2023-02-08T00:58:48Z

packages/labs/analyzer/src/test/javascript/functions_test.ts

+    try {
+      const packagePath = fileURLToPath(
+        new URL(`../../test-files/${lang}/functions`, import.meta.url).href
+      ) as AbsolutePath;
+      const analyzer = createPackageAnalyzer(packagePath);
+
+      const result = analyzer.getPackage();
+      const file = getSourceFilename('functions', lang);
+      const module = result.modules.find((m) => m.sourcePath === file);
+      if (module === undefined) {
+        throw new Error(`Analyzer did not analyze file '${file}'`);
+      }
+
+      ctx.packagePath = packagePath;
+      ctx.analyzer = analyzer;
+      ctx.module = module;
+    } catch (error) {
+      // Uvu has a bug where it silently ignores failures in before and after,
+      // see https://github.com/lukeed/uvu/issues/191.
+      console.error('uvu before error', error);
+      process.exit(1);
+    }


There's also a chunk like this in ./modules_test.ts. Probably not worth making a function yet if it's just the two, but are there others?

Will separate into follow-up PR.

https://github.com/lit/lit/pull/3661/files

justinfagnani · 2023-02-08T03:04:15Z

packages/labs/analyzer/src/test/javascript/functions_test.ts

+    assert.equal(fn.parameters?.[0].summary, undefined);
+    assert.equal(fn.parameters?.[0].type?.text, 'string');
+    assert.equal(fn.parameters?.[0].default, undefined);
+    assert.equal(fn.parameters?.[0].rest, false);


Uvu's equal() is a deep equal... can you use that for better readability?

It seems to do pretty bad things with classes, so gonna avoid for now.

Add support for analyzing function declarations

fb0459e

kevinpschaaf requested a review from bicknellr February 7, 2023 19:41

kevinpschaaf requested a review from justinfagnani as a code owner February 7, 2023 19:41

kevinpschaaf added 2 commits February 7, 2023 12:03

Add manifest emit for FunctionDeclaration

db897d1

Fix typo

06bc662

bicknellr reviewed Feb 8, 2023

View reviewed changes

justinfagnani approved these changes Feb 8, 2023

View reviewed changes

kevinpschaaf added 2 commits February 9, 2023 09:48

Merge branch 'main' into analyzer-fns

79a1028

Remove unused export

4ef6c0a

kevinpschaaf merged commit 7e20a52 into main Feb 10, 2023

kevinpschaaf deleted the analyzer-fns branch February 10, 2023 01:42

lit-robot mentioned this pull request Mar 10, 2023

Version Packages #3717

Merged

	export const getFunctionDeclaration = (
	const getFunctionDeclaration = (

Conversation

kevinpschaaf commented Feb 7, 2023

Uh oh!

changeset-bot bot commented Feb 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

github-actions bot commented Feb 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Tachometer Benchmark Results

Summary

nop-update

render

update

update-reflect

Results

render

update

update-reflect

render

update

nop-update

render

update

render

update

render

update

update-reflect

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

changeset-bot bot commented Feb 7, 2023 •

edited

Loading

github-actions bot commented Feb 7, 2023 •

edited

Loading