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

Use consistent prettier config for lit-html package #4749

Merged
merged 2 commits into from
Aug 28, 2024
Merged

Conversation

augustjk
Copy link
Contributor

@augustjk augustjk commented Aug 27, 2024

When prettier was updated in #4559 the default config for trailing comma changed from "es5" to "all", adding a lot of trailing comma to function params, specifically for the lit-html package as it had its own .prettierrc.json file without that option set.

This is inconsistent with the monorepo root prettierrc which explicitly had "trailingComma": "es5".

This PR adds the explicit trailing comma setting to packages/lit-html/src/.prettierrc.json.

Alternatively, I tried to remove the file and have the package use the root monorepo prettierrc but the "embeddedLanguageFormatting" setting was causing issue and test failures elsewhere.

Copy link

changeset-bot bot commented Aug 27, 2024

🦋 Changeset detected

Latest commit: fd9209a

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

This PR includes changesets to release 0 packages

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

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

Copy link
Contributor

github-actions bot commented Aug 27, 2024

📊 Tachometer Benchmark Results

Summary

nop-update

  • this-change, tip-of-tree, previous-release: unsure 🔍 -1% - +4% (-0.14ms - +0.53ms)
    this-change vs tip-of-tree

render

  • this-change: 47.81ms - 60.41ms
  • this-change, tip-of-tree, previous-release: unsure 🔍 -3% - +2% (-0.66ms - +0.31ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -1% - +1% (-0.37ms - +0.36ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -22% - +32% (-11.40ms - +17.39ms)
    this-change vs tip-of-tree

update

  • this-change: 488.70ms - 498.10ms
  • this-change, tip-of-tree, previous-release: unsure 🔍 -5% - +5% (-2.00ms - +2.06ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -1% - +1% (-0.55ms - +0.74ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -2% - +1% (-7.60ms - +7.32ms)
    this-change vs tip-of-tree

update-reflect

  • this-change: 489.14ms - 501.43ms
  • this-change, tip-of-tree, previous-release: unsure 🔍 -1% - +5% (-7.02ms - +26.21ms)
    this-change vs tip-of-tree

Results

this-change

render

VersionAvg timevs
47.81ms - 60.41ms-

update

VersionAvg timevs
488.70ms - 498.10ms-

update-reflect

VersionAvg timevs
489.14ms - 501.43ms-
this-change, tip-of-tree, previous-release

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
18.36ms - 19.04ms-unsure 🔍
-3% - +2%
-0.66ms - +0.31ms
unsure 🔍
-2% - +2%
-0.47ms - +0.45ms
tip-of-tree
tip-of-tree
18.53ms - 19.22msunsure 🔍
-2% - +4%
-0.31ms - +0.66ms
-unsure 🔍
-2% - +3%
-0.29ms - +0.63ms
previous-release
previous-release
18.39ms - 19.01msunsure 🔍
-2% - +2%
-0.45ms - +0.47ms
unsure 🔍
-3% - +2%
-0.63ms - +0.29ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
38.29ms - 41.00ms-unsure 🔍
-5% - +5%
-2.00ms - +2.06ms
unsure 🔍
-2% - +9%
-0.80ms - +3.24ms
tip-of-tree
tip-of-tree
38.11ms - 41.12msunsure 🔍
-5% - +5%
-2.06ms - +2.00ms
-unsure 🔍
-3% - +9%
-0.93ms - +3.32ms
previous-release
previous-release
36.92ms - 39.92msunsure 🔍
-8% - +2%
-3.24ms - +0.80ms
unsure 🔍
-8% - +2%
-3.32ms - +0.93ms
-

nop-update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
12.40ms - 12.80ms-unsure 🔍
-1% - +4%
-0.14ms - +0.53ms
unsure 🔍
-1% - +4%
-0.17ms - +0.47ms
tip-of-tree
tip-of-tree
12.14ms - 12.67msunsure 🔍
-4% - +1%
-0.53ms - +0.14ms
-unsure 🔍
-3% - +3%
-0.42ms - +0.32ms
previous-release
previous-release
12.19ms - 12.71msunsure 🔍
-4% - +1%
-0.47ms - +0.17ms
unsure 🔍
-3% - +3%
-0.32ms - +0.42ms
-
this-change, tip-of-tree, previous-release

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
34.43ms - 34.97ms-unsure 🔍
-1% - +1%
-0.37ms - +0.36ms
unsure 🔍
-1% - +1%
-0.27ms - +0.51ms
tip-of-tree
tip-of-tree
34.46ms - 34.95msunsure 🔍
-1% - +1%
-0.36ms - +0.37ms
-unsure 🔍
-1% - +1%
-0.25ms - +0.49ms
previous-release
previous-release
34.31ms - 34.87msunsure 🔍
-1% - +1%
-0.51ms - +0.27ms
unsure 🔍
-1% - +1%
-0.49ms - +0.25ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
68.13ms - 68.97ms-unsure 🔍
-1% - +1%
-0.55ms - +0.74ms
unsure 🔍
-0% - +1%
-0.33ms - +0.88ms
tip-of-tree
tip-of-tree
67.97ms - 68.94msunsure 🔍
-1% - +1%
-0.74ms - +0.55ms
-unsure 🔍
-1% - +1%
-0.47ms - +0.83ms
previous-release
previous-release
67.84ms - 68.71msunsure 🔍
-1% - +0%
-0.88ms - +0.33ms
unsure 🔍
-1% - +1%
-0.83ms - +0.47ms
-
this-change, tip-of-tree, previous-release

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
47.46ms - 68.15ms-unsure 🔍
-22% - +32%
-11.40ms - +17.39ms
unsure 🔍
-35% - +7%
-24.88ms - +5.54ms
tip-of-tree
tip-of-tree
44.80ms - 64.81msunsure 🔍
-29% - +19%
-17.39ms - +11.40ms
-unsure 🔍
-39% - +1%
-27.64ms - +2.31ms
previous-release
previous-release
56.33ms - 78.62msunsure 🔍
-12% - +45%
-5.54ms - +24.88ms
unsure 🔍
-7% - +53%
-2.31ms - +27.64ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
487.43ms - 498.24ms-unsure 🔍
-2% - +1%
-7.60ms - +7.32ms
unsure 🔍
-1% - +2%
-2.81ms - +11.02ms
tip-of-tree
tip-of-tree
487.83ms - 498.12msunsure 🔍
-1% - +2%
-7.32ms - +7.60ms
-unsure 🔍
-1% - +2%
-2.48ms - +10.96ms
previous-release
previous-release
484.42ms - 493.05msunsure 🔍
-2% - +1%
-11.02ms - +2.81ms
unsure 🔍
-2% - +0%
-10.96ms - +2.48ms
-

update-reflect

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
512.86ms - 545.10ms-unsure 🔍
-1% - +5%
-7.02ms - +26.21ms
unsure 🔍
-2% - +4%
-10.51ms - +22.76ms
tip-of-tree
tip-of-tree
515.37ms - 523.41msunsure 🔍
-5% - +1%
-26.21ms - +7.02ms
-unsure 🔍
-2% - +0%
-9.22ms - +2.28ms
previous-release
previous-release
518.75ms - 526.96msunsure 🔍
-4% - +2%
-22.76ms - +10.51ms
unsure 🔍
-0% - +2%
-2.28ms - +9.22ms
-

tachometer-reporter-action v2 for Benchmarks

Copy link
Contributor

The size of lit-html.js and lit-core.min.js are as expected.

@augustjk augustjk marked this pull request as draft August 27, 2024 21:27
@augustjk augustjk marked this pull request as ready for review August 27, 2024 21:54
Copy link
Contributor

@AndrewJakubowicz AndrewJakubowicz left a comment

Choose a reason for hiding this comment

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

Nice!

Copy link
Collaborator

@rictic rictic left a comment

Choose a reason for hiding this comment

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

Consistency is good. Started a discussion on comma policy in the discord, but getting consistency with other packages is an improvement.

@augustjk augustjk merged commit a2cd76c into main Aug 28, 2024
9 checks passed
@augustjk augustjk deleted the consistent-prettier branch August 28, 2024 17:04
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.

3 participants