Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[labs/analyzer] Add support for analyzing function declarations #3655

Merged
merged 5 commits into from Feb 10, 2023

Conversation

kevinpschaaf
Copy link
Member

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented Feb 7, 2023

🦋 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
Copy link
Contributor

github-actions bot commented Feb 7, 2023

📊 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

render

VersionAvg timevs
74.56ms - 79.03ms-

update

VersionAvg timevs
682.40ms - 693.35ms-

update-reflect

VersionAvg timevs
684.45ms - 691.70ms-
lit-html-kitchen-sink

render

VersionAvg timevs 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.70msunsure 🔍
-5% - +1%
-1.69ms - +0.45ms
-unsure 🔍
-4% - +4%
-1.12ms - +1.16ms
previous-release
previous-release
29.10ms - 31.01msunsure 🔍
-6% - +2%
-1.93ms - +0.65ms
unsure 🔍
-4% - +4%
-1.16ms - +1.12ms
-

update

VersionAvg timevs 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.34msunsure 🔍
-5% - +3%
-3.84ms - +2.12ms
-faster ✔
0% - 8%
0.29ms - 6.43ms
previous-release
previous-release
77.36ms - 81.58msunsure 🔍
-1% - +7%
-0.39ms - +5.39ms
slower ❌
0% - 9%
0.29ms - 6.43ms
-

nop-update

VersionAvg timevs 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.81msunsure 🔍
-8% - +10%
-2.00ms - +2.54ms
-unsure 🔍
-5% - +15%
-1.14ms - +3.64ms
previous-release
previous-release
22.19ms - 25.74msunsure 🔍
-13% - +6%
-3.38ms - +1.42ms
unsure 🔍
-14% - +4%
-3.64ms - +1.14ms
-
lit-html-repeat

render

VersionAvg timevs 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.35msunsure 🔍
-3% - +6%
-0.35ms - +0.69ms
-unsure 🔍
-3% - +6%
-0.35ms - +0.71ms
previous-release
previous-release
11.43ms - 12.17msunsure 🔍
-4% - +4%
-0.52ms - +0.51ms
unsure 🔍
-6% - +3%
-0.71ms - +0.35ms
-

update

VersionAvg timevs 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.24msunsure 🔍
-3% - +2%
-6.87ms - +4.30ms
-unsure 🔍
-3% - +1%
-7.31ms - +3.48ms
previous-release
previous-release
264.53ms - 272.53msunsure 🔍
-2% - +2%
-5.20ms - +6.46ms
unsure 🔍
-1% - +3%
-3.48ms - +7.31ms
-
lit-html-template-heavy

render

VersionAvg timevs 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.31msunsure 🔍
-2% - +2%
-1.07ms - +1.30ms
-unsure 🔍
-1% - +3%
-0.62ms - +1.70ms
previous-release
previous-release
51.13ms - 52.70msunsure 🔍
-3% - +1%
-1.56ms - +0.71ms
unsure 🔍
-3% - +1%
-1.70ms - +0.62ms
-

update

VersionAvg timevs 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.03msunsure 🔍
-1% - +0%
-0.81ms - +0.50ms
-unsure 🔍
-0% - +1%
-0.28ms - +0.90ms
previous-release
previous-release
103.88ms - 104.71msunsure 🔍
-1% - +0%
-1.11ms - +0.18ms
unsure 🔍
-1% - +0%
-0.90ms - +0.28ms
-
reactive-element-list

render

VersionAvg timevs 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.26msunsure 🔍
-3% - +1%
-1.59ms - +0.71ms
-unsure 🔍
-2% - +2%
-1.08ms - +1.08ms
previous-release
previous-release
48.74ms - 50.38msunsure 🔍
-3% - +2%
-1.66ms - +0.79ms
unsure 🔍
-2% - +2%
-1.08ms - +1.08ms
-

update

VersionAvg timevs 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.37msunsure 🔍
-1% - +0%
-3.48ms - +2.87ms
-unsure 🔍
-1% - +1%
-3.50ms - +3.56ms
previous-release
previous-release
686.35ms - 691.55msunsure 🔍
-1% - +0%
-3.67ms - +3.01ms
unsure 🔍
-1% - +1%
-3.56ms - +3.50ms
-

update-reflect

VersionAvg timevs 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.50msunsure 🔍
-0% - +1%
-2.27ms - +4.23ms
-unsure 🔍
-1% - +0%
-3.61ms - +3.53ms
previous-release
previous-release
718.49ms - 723.73msunsure 🔍
-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 Outdated Show resolved Hide resolved
};
};

export const getFunctionDeclaration = (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export const getFunctionDeclaration = (
const getFunctionDeclaration = (

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getFunctionLikeInfo() is called here and in ClassMethod

Comment on lines +28 to +49
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);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will separate into follow-up PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@kevinpschaaf kevinpschaaf merged commit 7e20a52 into main Feb 10, 2023
@kevinpschaaf kevinpschaaf deleted the analyzer-fns branch February 10, 2023 01:42
@lit-robot lit-robot mentioned this pull request Mar 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants