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

Add missing dependency to eslint-plugin-lit #4545

Merged
merged 2 commits into from
Feb 13, 2024
Merged

Conversation

justinfagnani
Copy link
Collaborator

@justinfagnani justinfagnani commented Feb 9, 2024

I caught this running lint locally. Not sure why it wasn't flagged on CI.

Edit(ajakubowicz):

  • Applied an additional commit which fixes two wireit issues. 1. eslint-plugin should build the lit analyzer as a wireit dependency. 2. The lit analyzer wireit config has a subtle issue where running build after build:ts-bundle will delete the typescript.js file, because it's captured by the build outputs. Fix is to ignore it.

Copy link
Contributor

github-actions bot commented Feb 9, 2024

📊 Tachometer Benchmark Results

Summary

nop-update

  • this-change, tip-of-tree, previous-release: unsure 🔍 -5% - +4% (-0.62ms - +0.46ms)
    this-change vs tip-of-tree

render

  • this-change: 46.55ms - 48.48ms
  • this-change, tip-of-tree, previous-release: unsure 🔍 -2% - +6% (-0.42ms - +1.02ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -1% - +3% (-0.45ms - +0.91ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: slower ❌ 0% - 4% (0.03ms - 1.17ms)
    this-change vs tip-of-tree

update

  • this-change: 520.54ms - 528.65ms
  • this-change, tip-of-tree, previous-release: unsure 🔍 -8% - +5% (-3.23ms - +2.16ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -0% - +2% (-0.33ms - +1.73ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -1% - +1% (-4.62ms - +5.20ms)
    this-change vs tip-of-tree

update-reflect

  • this-change: 521.55ms - 528.54ms
  • this-change, tip-of-tree, previous-release: unsure 🔍 -1% - +1% (-5.00ms - +4.08ms)
    this-change vs tip-of-tree

Results

this-change

render

VersionAvg timevs
46.55ms - 48.48ms-

update

VersionAvg timevs
520.54ms - 528.65ms-

update-reflect

VersionAvg timevs
521.55ms - 528.54ms-
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.34ms - 19.46ms-unsure 🔍
-2% - +6%
-0.42ms - +1.02ms
unsure 🔍
-6% - +3%
-1.06ms - +0.62ms
tip-of-tree
tip-of-tree
18.14ms - 19.06msunsure 🔍
-5% - +2%
-1.02ms - +0.42ms
-unsure 🔍
-7% - +1%
-1.30ms - +0.25ms
previous-release
previous-release
18.49ms - 19.75msunsure 🔍
-3% - +6%
-0.62ms - +1.06ms
unsure 🔍
-1% - +7%
-0.25ms - +1.30ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
39.11ms - 42.94ms-unsure 🔍
-8% - +5%
-3.23ms - +2.16ms
unsure 🔍
-8% - +8%
-3.44ms - +3.42ms
tip-of-tree
tip-of-tree
39.67ms - 43.45msunsure 🔍
-5% - +8%
-2.16ms - +3.23ms
-unsure 🔍
-7% - +10%
-2.89ms - +3.94ms
previous-release
previous-release
38.19ms - 43.88msunsure 🔍
-8% - +8%
-3.42ms - +3.44ms
unsure 🔍
-9% - +7%
-3.94ms - +2.89ms
-

nop-update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
10.97ms - 11.76ms-unsure 🔍
-5% - +4%
-0.62ms - +0.46ms
unsure 🔍
-2% - +9%
-0.25ms - +1.02ms
tip-of-tree
tip-of-tree
11.08ms - 11.80msunsure 🔍
-4% - +5%
-0.46ms - +0.62ms
-unsure 🔍
-1% - +10%
-0.15ms - +1.08ms
previous-release
previous-release
10.48ms - 11.47msunsure 🔍
-9% - +2%
-1.02ms - +0.25ms
unsure 🔍
-9% - +1%
-1.08ms - +0.15ms
-
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
33.28ms - 34.34ms-unsure 🔍
-1% - +3%
-0.45ms - +0.91ms
unsure 🔍
-1% - +3%
-0.50ms - +0.89ms
tip-of-tree
tip-of-tree
33.15ms - 34.00msunsure 🔍
-3% - +1%
-0.91ms - +0.45ms
-unsure 🔍
-2% - +2%
-0.65ms - +0.58ms
previous-release
previous-release
33.17ms - 34.06msunsure 🔍
-3% - +1%
-0.89ms - +0.50ms
unsure 🔍
-2% - +2%
-0.58ms - +0.65ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
70.05ms - 71.70ms-unsure 🔍
-0% - +2%
-0.33ms - +1.73ms
unsure 🔍
-1% - +3%
-0.35ms - +1.84ms
tip-of-tree
tip-of-tree
69.56ms - 70.80msunsure 🔍
-2% - +0%
-1.73ms - +0.33ms
-unsure 🔍
-1% - +1%
-0.91ms - +1.00ms
previous-release
previous-release
69.40ms - 70.85msunsure 🔍
-3% - +0%
-1.84ms - +0.35ms
unsure 🔍
-1% - +1%
-1.00ms - +0.91ms
-
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
31.65ms - 32.50ms-slower ❌
0% - 4%
0.03ms - 1.17ms
unsure 🔍
-0% - +3%
-0.09ms - +1.03ms
tip-of-tree
tip-of-tree
31.10ms - 31.86msfaster ✔
0% - 4%
0.03ms - 1.17ms
-unsure 🔍
-2% - +1%
-0.65ms - +0.40ms
previous-release
previous-release
31.25ms - 31.96msunsure 🔍
-3% - +0%
-1.03ms - +0.09ms
unsure 🔍
-1% - +2%
-0.40ms - +0.65ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
529.84ms - 537.28ms-unsure 🔍
-1% - +1%
-4.62ms - +5.20ms
unsure 🔍
-1% - +1%
-5.22ms - +5.30ms
tip-of-tree
tip-of-tree
530.06ms - 536.49msunsure 🔍
-1% - +1%
-5.20ms - +4.62ms
-unsure 🔍
-1% - +1%
-5.17ms - +4.66ms
previous-release
previous-release
529.81ms - 537.25msunsure 🔍
-1% - +1%
-5.30ms - +5.22ms
unsure 🔍
-1% - +1%
-4.66ms - +5.17ms
-

update-reflect

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
535.62ms - 541.76ms-unsure 🔍
-1% - +1%
-5.00ms - +4.08ms
unsure 🔍
-1% - +1%
-5.87ms - +4.68ms
tip-of-tree
tip-of-tree
535.81ms - 542.49msunsure 🔍
-1% - +1%
-4.08ms - +5.00ms
-unsure 🔍
-1% - +1%
-5.57ms - +5.30ms
previous-release
previous-release
535.00ms - 543.58msunsure 🔍
-1% - +1%
-4.68ms - +5.87ms
unsure 🔍
-1% - +1%
-5.30ms - +5.57ms
-

tachometer-reporter-action v2 for Benchmarks

Copy link

changeset-bot bot commented Feb 9, 2024

🦋 Changeset detected

Latest commit: ab7069d

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 Feb 9, 2024

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

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.

It's interesting that it wasn't caught on CI. Is it possible that it implicitly works because lit-labs/analyzer is built by other tests.

A similar dependency that may be missing is a wireit dep on the analyzer build command as a dependency from the eslint-plugin build command.

Pre-approving!

Edit: I applied wireit fixes.

@AndrewJakubowicz AndrewJakubowicz merged commit b779807 into main Feb 13, 2024
9 checks passed
@AndrewJakubowicz AndrewJakubowicz deleted the fix-linter-deps branch February 13, 2024 18:23
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