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

[reactive-element] Fix Node build auto-shimming of HTMLElement to respect existing global shim #3561

Merged
merged 4 commits into from Jan 12, 2023

Conversation

augustjk
Copy link
Member

Fixes #3559

The replace of extends HTMLElement with extends (globalThis.HTMLElement ?? HTMLElement) was not happening due to the includes option for @rollup/plugin-replace not working properly. Updating the version fixes this.

Note: To keep a single version of the plugin in the monorepo, the version was also updated for the starter kits.

Now the outputs look like:

Dev Node build:

class ReactiveElement
 extends (globalThis.HTMLElement ?? HTMLElement) {

Prod Node build:

class v extends(globalThis.HTMLElement??i)

I added another test in the vein of the node-import tests. It feels like there are a lot of scripts for the node tests now but unsure if there's a better way to do this.

@changeset-bot
Copy link

changeset-bot bot commented Jan 11, 2023

🦋 Changeset detected

Latest commit: 202d93f

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

This PR includes changesets to release 4 packages
Name Type
@lit/lit-starter-js Patch
@lit/lit-starter-ts Patch
lit Patch
@lit/reactive-element 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 Jan 11, 2023

📊 Tachometer Benchmark Results

Summary

nop-update

  • lit-html-kitchen-sink: unsure 🔍 -5% - +3% (-1.22ms - +0.71ms)
    this-change vs tip-of-tree

render

  • lit-element-list: 81.73ms - 87.68ms
  • lit-html-kitchen-sink: unsure 🔍 -6% - +5% (-1.83ms - +1.46ms)
    this-change vs tip-of-tree
  • lit-html-repeat: unsure 🔍 -8% - +1% (-0.95ms - +0.14ms)
    this-change vs tip-of-tree
  • lit-html-template-heavy: unsure 🔍 -3% - +2% (-1.39ms - +1.00ms)
    this-change vs tip-of-tree
  • reactive-element-list: unsure 🔍 -3% - +3% (-1.35ms - +1.48ms)
    this-change vs tip-of-tree

update

  • lit-element-list: 691.60ms - 703.67ms
  • lit-html-kitchen-sink: faster ✔ 1% - 8% (0.43ms - 6.79ms)
    this-change vs tip-of-tree
  • lit-html-repeat: unsure 🔍 -5% - +1% (-12.90ms - +3.07ms)
    this-change vs tip-of-tree
  • lit-html-template-heavy: unsure 🔍 -1% - +2% (-0.56ms - +1.61ms)
    this-change vs tip-of-tree
  • reactive-element-list: unsure 🔍 -0% - +1% (-1.84ms - +5.69ms)
    this-change vs tip-of-tree

update-reflect

  • lit-element-list: 697.30ms - 704.35ms
  • reactive-element-list: unsure 🔍 -1% - +1% (-4.03ms - +5.00ms)
    this-change vs tip-of-tree

Results

lit-element-list

render

VersionAvg timevs
81.73ms - 87.68ms-

update

VersionAvg timevs
691.60ms - 703.67ms-

update-reflect

VersionAvg timevs
697.30ms - 704.35ms-
lit-html-kitchen-sink

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
30.53ms - 32.55ms-unsure 🔍
-6% - +5%
-1.83ms - +1.46ms
unsure 🔍
-4% - +8%
-1.06ms - +2.38ms
tip-of-tree
tip-of-tree
30.43ms - 33.03msunsure 🔍
-5% - +6%
-1.46ms - +1.83ms
-unsure 🔍
-4% - +9%
-1.05ms - +2.75ms
previous-release
previous-release
29.49ms - 32.27msunsure 🔍
-8% - +3%
-2.38ms - +1.06ms
unsure 🔍
-9% - +3%
-2.75ms - +1.05ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
76.75ms - 81.10ms-faster ✔
1% - 8%
0.43ms - 6.79ms
unsure 🔍
-6% - +1%
-5.03ms - +0.82ms
tip-of-tree
tip-of-tree
80.22ms - 84.85msslower ❌
0% - 9%
0.43ms - 6.79ms
-unsure 🔍
-2% - +6%
-1.53ms - +4.54ms
previous-release
previous-release
79.08ms - 82.98msunsure 🔍
-1% - +6%
-0.82ms - +5.03ms
unsure 🔍
-5% - +2%
-4.54ms - +1.53ms
-

nop-update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
22.14ms - 23.08ms-unsure 🔍
-5% - +3%
-1.22ms - +0.71ms
unsure 🔍
-3% - +4%
-0.60ms - +0.84ms
tip-of-tree
tip-of-tree
22.02ms - 23.70msunsure 🔍
-3% - +5%
-0.71ms - +1.22ms
-unsure 🔍
-3% - +6%
-0.63ms - +1.38ms
previous-release
previous-release
21.94ms - 23.03msunsure 🔍
-4% - +3%
-0.84ms - +0.60ms
unsure 🔍
-6% - +3%
-1.38ms - +0.63ms
-
lit-html-repeat

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
11.47ms - 12.26ms-unsure 🔍
-8% - +1%
-0.95ms - +0.14ms
unsure 🔍
-6% - +3%
-0.79ms - +0.32ms
tip-of-tree
tip-of-tree
11.90ms - 12.64msunsure 🔍
-1% - +8%
-0.14ms - +0.95ms
-unsure 🔍
-3% - +6%
-0.37ms - +0.71ms
previous-release
previous-release
11.71ms - 12.49msunsure 🔍
-3% - +7%
-0.32ms - +0.79ms
unsure 🔍
-6% - +3%
-0.71ms - +0.37ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
261.59ms - 271.58ms-unsure 🔍
-5% - +1%
-12.90ms - +3.07ms
unsure 🔍
-4% - +2%
-11.77ms - +5.52ms
tip-of-tree
tip-of-tree
265.27ms - 277.73msunsure 🔍
-1% - +5%
-3.07ms - +12.90ms
-unsure 🔍
-3% - +4%
-7.62ms - +11.20ms
previous-release
previous-release
262.65ms - 276.77msunsure 🔍
-2% - +4%
-5.52ms - +11.77ms
unsure 🔍
-4% - +3%
-11.20ms - +7.62ms
-
lit-html-template-heavy

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
52.13ms - 53.57ms-unsure 🔍
-3% - +2%
-1.39ms - +1.00ms
unsure 🔍
-1% - +2%
-0.75ms - +1.19ms
tip-of-tree
tip-of-tree
52.09ms - 54.00msunsure 🔍
-2% - +3%
-1.00ms - +1.39ms
-unsure 🔍
-1% - +3%
-0.74ms - +1.57ms
previous-release
previous-release
51.98ms - 53.28msunsure 🔍
-2% - +1%
-1.19ms - +0.75ms
unsure 🔍
-3% - +1%
-1.57ms - +0.74ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
104.25ms - 105.99ms-unsure 🔍
-1% - +2%
-0.56ms - +1.61ms
unsure 🔍
-1% - +1%
-0.74ms - +1.44ms
tip-of-tree
tip-of-tree
103.95ms - 105.24msunsure 🔍
-2% - +1%
-1.61ms - +0.56ms
-unsure 🔍
-1% - +1%
-1.10ms - +0.75ms
previous-release
previous-release
104.11ms - 105.43msunsure 🔍
-1% - +1%
-1.44ms - +0.74ms
unsure 🔍
-1% - +1%
-0.75ms - +1.10ms
-
reactive-element-list

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
52.94ms - 54.81ms-unsure 🔍
-3% - +3%
-1.35ms - +1.48ms
unsure 🔍
-2% - +3%
-1.31ms - +1.36ms
tip-of-tree
tip-of-tree
52.75ms - 54.87msunsure 🔍
-3% - +3%
-1.48ms - +1.35ms
-unsure 🔍
-3% - +3%
-1.46ms - +1.39ms
previous-release
previous-release
52.89ms - 54.80msunsure 🔍
-3% - +2%
-1.36ms - +1.31ms
unsure 🔍
-3% - +3%
-1.39ms - +1.46ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
685.68ms - 690.93ms-unsure 🔍
-0% - +1%
-1.84ms - +5.69ms
unsure 🔍
-1% - +0%
-5.02ms - +2.94ms
tip-of-tree
tip-of-tree
683.68ms - 689.07msunsure 🔍
-1% - +0%
-5.69ms - +1.84ms
-unsure 🔍
-1% - +0%
-6.99ms - +1.06ms
previous-release
previous-release
686.35ms - 692.33msunsure 🔍
-0% - +1%
-2.94ms - +5.02ms
unsure 🔍
-0% - +1%
-1.06ms - +6.99ms
-

update-reflect

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
717.42ms - 723.36ms-unsure 🔍
-1% - +1%
-4.03ms - +5.00ms
unsure 🔍
-1% - +1%
-4.62ms - +4.38ms
tip-of-tree
tip-of-tree
716.51ms - 723.30msunsure 🔍
-1% - +1%
-5.00ms - +4.03ms
-unsure 🔍
-1% - +1%
-5.39ms - +4.19ms
previous-release
previous-release
717.13ms - 723.89msunsure 🔍
-1% - +1%
-4.38ms - +4.62ms
unsure 🔍
-1% - +1%
-4.19ms - +5.39ms
-

tachometer-reporter-action v2 for Benchmarks

Copy link
Member

@aomarks aomarks left a comment

Choose a reason for hiding this comment

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

Great!

.changeset/tidy-lamps-rest.md Outdated Show resolved Hide resolved
Co-authored-by: Alexander Marks <aomarks@google.com>
@augustjk
Copy link
Member Author

Filed #3562 for better handling of the node tests.

@augustjk augustjk enabled auto-merge (squash) January 12, 2023 01:11
@augustjk augustjk merged commit e5c254e into main Jan 12, 2023
@augustjk augustjk deleted the fix-node-html-shim branch January 12, 2023 01:22
@lit-robot lit-robot mentioned this pull request Jan 12, 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.

ssr-dom-shim in reactive-element is breaking test cases in vitest/happy-dom
2 participants