[labs/analyzer] Don't extract JSDoc types in TS. Port type tests to JS. #3658

bicknellr · 2023-02-08T23:06:16Z

TS doesn't consider types provided in JSDoc comments as having any effect in .ts files, even though they're still accessible in the AST. This PR updates the analyzer to match this behavior and also copies/ports the type tests to JS to confirm that JSDoc comments still affect the analysis of .js files.

This PR is easier to read with leading whitespace removed.

…if applicable. (...) When TypeScript is checking `.ts` files, JSDoc comments that indicate types have no effect despite being parsed and reachable through the AST. When checking `.js` files, JSDoc comments are automatically considered by `tsc` and don't need to be separately taken into account.

changeset-bot · 2023-02-08T23:06:20Z

🦋 Changeset detected

Latest commit: 70e8068

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-08T23:06:38Z

📊 Tachometer Benchmark Results

Summary

nop-update

lit-html-kitchen-sink: unsure 🔍 -1% - +18% (-0.21ms - +3.97ms)
^{this-change vs tip-of-tree}

render

lit-element-list: 72.95ms - 75.86ms
lit-html-kitchen-sink: unsure 🔍 -3% - +6% (-0.80ms - +1.92ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -11% - +2% (-1.33ms - +0.31ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -5% - +1% (-2.75ms - +0.39ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -2% - +3% (-0.83ms - +1.32ms)
^{this-change vs tip-of-tree}

update

lit-element-list: 679.88ms - 688.82ms
lit-html-kitchen-sink: unsure 🔍 -4% - +6% (-2.73ms - +4.27ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -15% - +3% (-45.27ms - +9.64ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -2% - +2% (-1.75ms - +1.98ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -1% - +1% (-7.46ms - +7.66ms)
^{this-change vs tip-of-tree}

update-reflect

lit-element-list: 683.85ms - 691.67ms
reactive-element-list: unsure 🔍 -2% - +0% (-14.24ms - +0.53ms)
^{this-change vs tip-of-tree}

Results

lit-element-list

Browser: chrome-headless 109.0.5414.119
Sample size: 50
Built by: Benchmarks #4409
Commit: f3c089d

render

Version	Avg time	vs
	72.95ms - 75.86ms	-

update

Version	Avg time	vs
	679.88ms - 688.82ms	-

update-reflect

Version	Avg time	vs
	683.85ms - 691.67ms	-

lit-html-kitchen-sink

Browser: chrome-headless 109.0.5414.119
Sample size: 30
Built by: Benchmarks #4409
Commit: f3c089d

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	30.01ms - 32.32ms	-	unsure 🔍 -3% - +6% -0.80ms - +1.92ms	unsure 🔍 -5% - +5% -1.52ms - +1.59ms
tip-of-tree tip-of-tree	29.89ms - 31.32ms	unsure 🔍 -6% - +3% -1.92ms - +0.80ms	-	unsure 🔍 -6% - +2% -1.78ms - +0.73ms
previous-release previous-release	30.09ms - 32.17ms	unsure 🔍 -5% - +5% -1.59ms - +1.52ms	unsure 🔍 -2% - +6% -0.73ms - +1.78ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	74.79ms - 79.96ms	-	unsure 🔍 -4% - +6% -2.73ms - +4.27ms	unsure 🔍 -5% - +4% -4.06ms - +3.22ms
tip-of-tree tip-of-tree	74.25ms - 78.96ms	unsure 🔍 -5% - +4% -4.27ms - +2.73ms	-	unsure 🔍 -6% - +3% -4.67ms - +2.29ms
previous-release previous-release	75.23ms - 80.36ms	unsure 🔍 -4% - +5% -3.22ms - +4.06ms	unsure 🔍 -3% - +6% -2.29ms - +4.67ms	-

nop-update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	22.91ms - 25.89ms	-	unsure 🔍 -1% - +18% -0.21ms - +3.97ms	unsure 🔍 -9% - +8% -2.32ms - +1.88ms
tip-of-tree tip-of-tree	21.06ms - 23.98ms	unsure 🔍 -16% - +1% -3.97ms - +0.21ms	-	faster ✔ 0% - 17% 0.02ms - 4.18ms
previous-release previous-release	23.14ms - 26.10ms	unsure 🔍 -8% - +10% -1.88ms - +2.32ms	unsure 🔍 -0% - +19% +0.02ms - +4.18ms	-

lit-html-repeat

Browser: chrome-headless 109.0.5414.119
Sample size: 30
Built by: Benchmarks #4409
Commit: f3c089d

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	11.27ms - 12.11ms	-	unsure 🔍 -11% - +2% -1.33ms - +0.31ms	unsure 🔍 -7% - +3% -0.78ms - +0.35ms
tip-of-tree tip-of-tree	11.50ms - 12.91ms	unsure 🔍 -3% - +11% -0.31ms - +1.33ms	-	unsure 🔍 -4% - +9% -0.50ms - +1.10ms
previous-release previous-release	11.53ms - 12.28ms	unsure 🔍 -3% - +7% -0.35ms - +0.78ms	unsure 🔍 -9% - +4% -1.10ms - +0.50ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	270.91ms - 303.15ms	-	unsure 🔍 -15% - +3% -45.27ms - +9.64ms	unsure 🔍 -15% - +4% -45.24ms - +13.94ms
tip-of-tree tip-of-tree	282.62ms - 327.06ms	unsure 🔍 -4% - +16% -9.64ms - +45.27ms	-	unsure 🔍 -10% - +12% -31.14ms - +35.48ms
previous-release previous-release	277.86ms - 327.49ms	unsure 🔍 -5% - +16% -13.94ms - +45.24ms	unsure 🔍 -12% - +10% -35.48ms - +31.14ms	-

lit-html-template-heavy

Browser: chrome-headless 109.0.5414.119
Sample size: 50
Built by: Benchmarks #4409
Commit: f3c089d

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	51.25ms - 53.15ms	-	unsure 🔍 -5% - +1% -2.75ms - +0.39ms	unsure 🔍 -4% - +3% -2.04ms - +1.40ms
tip-of-tree tip-of-tree	52.13ms - 54.63ms	unsure 🔍 -1% - +5% -0.39ms - +2.75ms	-	unsure 🔍 -2% - +5% -1.04ms - +2.76ms
previous-release previous-release	51.09ms - 53.95ms	unsure 🔍 -3% - +4% -1.40ms - +2.04ms	unsure 🔍 -5% - +2% -2.76ms - +1.04ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	104.99ms - 107.66ms	-	unsure 🔍 -2% - +2% -1.75ms - +1.98ms	unsure 🔍 -0% - +3% -0.32ms - +2.87ms
tip-of-tree tip-of-tree	104.91ms - 107.51ms	unsure 🔍 -2% - +2% -1.98ms - +1.75ms	-	unsure 🔍 -0% - +3% -0.41ms - +2.72ms
previous-release previous-release	104.18ms - 105.92ms	unsure 🔍 -3% - +0% -2.87ms - +0.32ms	unsure 🔍 -3% - +0% -2.72ms - +0.41ms	-

reactive-element-list

Browser: chrome-headless 109.0.5414.119
Sample size: 50
Built by: Benchmarks #4409
Commit: f3c089d

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	48.69ms - 50.21ms	-	unsure 🔍 -2% - +3% -0.83ms - +1.32ms	unsure 🔍 -3% - +2% -1.42ms - +0.78ms
tip-of-tree tip-of-tree	48.44ms - 49.97ms	unsure 🔍 -3% - +2% -1.32ms - +0.83ms	-	unsure 🔍 -3% - +1% -1.67ms - +0.53ms
previous-release previous-release	48.98ms - 50.57ms	unsure 🔍 -2% - +3% -0.78ms - +1.42ms	unsure 🔍 -1% - +3% -0.53ms - +1.67ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	686.04ms - 696.82ms	-	unsure 🔍 -1% - +1% -7.46ms - +7.66ms	unsure 🔍 -1% - +1% -5.17ms - +9.16ms
tip-of-tree tip-of-tree	686.04ms - 696.62ms	unsure 🔍 -1% - +1% -7.66ms - +7.46ms	-	unsure 🔍 -1% - +1% -5.20ms - +8.99ms
previous-release previous-release	684.71ms - 694.16ms	unsure 🔍 -1% - +1% -9.16ms - +5.17ms	unsure 🔍 -1% - +1% -8.99ms - +5.20ms	-

update-reflect

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	712.76ms - 719.69ms	-	unsure 🔍 -2% - +0% -14.24ms - +0.53ms	unsure 🔍 -1% - +0% -10.00ms - +2.88ms
tip-of-tree tip-of-tree	716.56ms - 729.60ms	unsure 🔍 -0% - +2% -0.53ms - +14.24ms	-	unsure 🔍 -1% - +2% -5.19ms - +11.78ms
previous-release previous-release	714.35ms - 725.21ms	unsure 🔍 -0% - +1% -2.88ms - +10.00ms	unsure 🔍 -2% - +1% -11.78ms - +5.19ms	-

_{tachometer-reporter-action v2 for Benchmarks}

These tests were checking if types defined by JSDoc comments in `.ts` files were being output by the analyzer. However, TS does not consider JSDoc comments to have any effect in `.ts` files, even though they're parsed and reachable in the AST.

packages/labs/analyzer/src/lib/types.ts

bicknellr added 2 commits February 8, 2023 15:07

Remove incorrect JSDoc type tests. (...)

0c39638

These tests were checking if types defined by JSDoc comments in `.ts` files were being output by the analyzer. However, TS does not consider JSDoc comments to have any effect in `.ts` files, even though they're parsed and reachable in the AST.

Copy TS type tests and port to JS.

c98dab8

bicknellr force-pushed the analyzer-jsdoc-types-tests branch from a76ca1e to c98dab8 Compare February 8, 2023 23:09

bicknellr changed the title ~~[labs/analyzer] Port type tests to JS.~~ [labs/analyzer] Don't extract JSDoc types in TS. Port type tests to JS. Feb 8, 2023

bicknellr commented Feb 8, 2023

View reviewed changes

packages/labs/analyzer/src/lib/types.ts Outdated Show resolved Hide resolved

bicknellr added 2 commits February 8, 2023 15:21

Remove a comment that no longer applies.

8849501

Add a changeset.

70e8068

bicknellr marked this pull request as ready for review February 8, 2023 23:34

bicknellr requested a review from justinfagnani as a code owner February 8, 2023 23:34

bicknellr requested a review from kevinpschaaf February 8, 2023 23:34

justinfagnani approved these changes Feb 8, 2023

View reviewed changes

kevinpschaaf approved these changes Feb 9, 2023

View reviewed changes

bicknellr merged commit b7b01c0 into main Feb 9, 2023

bicknellr deleted the analyzer-jsdoc-types-tests branch February 9, 2023 01:15

lit-robot mentioned this pull request Mar 10, 2023

Version Packages #3717

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[labs/analyzer] Don't extract JSDoc types in TS. Port type tests to JS. #3658

[labs/analyzer] Don't extract JSDoc types in TS. Port type tests to JS. #3658

bicknellr commented Feb 8, 2023 •

edited

changeset-bot bot commented Feb 8, 2023 •

edited

github-actions bot commented Feb 8, 2023 •

edited

📊 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

[labs/analyzer] Don't extract JSDoc types in TS. Port type tests to JS. #3658

[labs/analyzer] Don't extract JSDoc types in TS. Port type tests to JS. #3658

Conversation

bicknellr commented Feb 8, 2023 • edited

changeset-bot bot commented Feb 8, 2023 • edited

🦋 Changeset detected

github-actions bot commented Feb 8, 2023 • edited

📊 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

bicknellr commented Feb 8, 2023 •

edited

changeset-bot bot commented Feb 8, 2023 •

edited

github-actions bot commented Feb 8, 2023 •

edited