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

[core] Fix CSSStyleSheet is not defined error in Node build #3222

Merged
merged 3 commits into from Aug 18, 2022

Conversation

aomarks
Copy link
Member

@aomarks aomarks commented Aug 17, 2022

The recently added node build would crash when importing a component that had both static styles and used the @property decorator. That's because the @property decorator causes Lit class initialization at module load time, which ran an instanceof CSSStyleSheet check before this PR. Since CSSStyleSheet is not defined by default, the error would occur.

This PR fixes that by not assuming CSSStyleSheet will be defined when in Node mode.

Fixes #3218

@changeset-bot
Copy link

changeset-bot bot commented Aug 17, 2022

🦋 Changeset detected

Latest commit: 75ac22f

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

This PR includes changesets to release 2 packages
Name Type
@lit/reactive-element Patch
lit 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 Aug 17, 2022

📊 Tachometer Benchmark Results

Summary

nop-update

  • lit-html-kitchen-sink: unsure 🔍 -4% - +4% (-1.20ms - +1.31ms)
    this-change vs tip-of-tree

render

  • lit-element-list: 107.72ms - 112.23ms
  • lit-html-kitchen-sink: unsure 🔍 -2% - +3% (-1.08ms - +1.24ms)
    this-change vs tip-of-tree
  • lit-html-repeat: unsure 🔍 -3% - +2% (-0.37ms - +0.28ms)
    this-change vs tip-of-tree
  • lit-html-template-heavy: unsure 🔍 -5% - +1% (-3.45ms - +0.72ms)
    this-change vs tip-of-tree
  • reactive-element-list: unsure 🔍 -2% - +2% (-1.24ms - +1.77ms)
    this-change vs tip-of-tree

update

  • lit-element-list: 1128.42ms - 1141.16ms
  • lit-html-kitchen-sink: unsure 🔍 -2% - +3% (-2.29ms - +3.80ms)
    this-change vs tip-of-tree
  • lit-html-repeat: unsure 🔍 -1% - +2% (-4.06ms - +6.71ms)
    this-change vs tip-of-tree
  • lit-html-template-heavy: unsure 🔍 -2% - +2% (-3.14ms - +3.96ms)
    this-change vs tip-of-tree
  • reactive-element-list: slower ❌ 0% - 1% (0.49ms - 16.72ms)
    this-change vs tip-of-tree

update-reflect

  • lit-element-list: 1153.02ms - 1164.11ms
  • reactive-element-list: unsure 🔍 -1% - +1% (-7.63ms - +10.12ms)
    this-change vs tip-of-tree

Results

lit-element-list

render

VersionAvg timevs
107.72ms - 112.23ms-

update

VersionAvg timevs
1128.42ms - 1141.16ms-

update-reflect

VersionAvg timevs
1153.02ms - 1164.11ms-
lit-html-kitchen-sink

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
42.89ms - 44.53ms-unsure 🔍
-2% - +3%
-1.08ms - +1.24ms
unsure 🔍
-0% - +4%
-0.13ms - +1.67ms
tip-of-tree
tip-of-tree
42.81ms - 44.45msunsure 🔍
-3% - +2%
-1.24ms - +1.08ms
-unsure 🔍
-1% - +4%
-0.22ms - +1.59ms
previous-release
previous-release
42.57ms - 43.32msunsure 🔍
-4% - +0%
-1.67ms - +0.13ms
unsure 🔍
-4% - +0%
-1.59ms - +0.22ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
118.01ms - 121.49ms-unsure 🔍
-2% - +3%
-2.29ms - +3.80ms
unsure 🔍
-3% - +2%
-4.04ms - +2.59ms
tip-of-tree
tip-of-tree
116.49ms - 121.50msunsure 🔍
-3% - +2%
-3.80ms - +2.29ms
-unsure 🔍
-4% - +2%
-5.25ms - +2.29ms
previous-release
previous-release
117.65ms - 123.30msunsure 🔍
-2% - +3%
-2.59ms - +4.04ms
unsure 🔍
-2% - +4%
-2.29ms - +5.25ms
-

nop-update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
31.17ms - 33.21ms-unsure 🔍
-4% - +4%
-1.20ms - +1.31ms
unsure 🔍
-6% - +2%
-2.05ms - +0.66ms
tip-of-tree
tip-of-tree
31.39ms - 32.88msunsure 🔍
-4% - +4%
-1.31ms - +1.20ms
-unsure 🔍
-6% - +1%
-1.92ms - +0.42ms
previous-release
previous-release
31.98ms - 33.78msunsure 🔍
-2% - +6%
-0.66ms - +2.05ms
unsure 🔍
-1% - +6%
-0.42ms - +1.92ms
-
lit-html-repeat

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
12.89ms - 13.33ms-unsure 🔍
-3% - +2%
-0.37ms - +0.28ms
unsure 🔍
-3% - +2%
-0.43ms - +0.31ms
tip-of-tree
tip-of-tree
12.92ms - 13.39msunsure 🔍
-2% - +3%
-0.28ms - +0.37ms
-unsure 🔍
-3% - +3%
-0.39ms - +0.37ms
previous-release
previous-release
12.87ms - 13.47msunsure 🔍
-2% - +3%
-0.31ms - +0.43ms
unsure 🔍
-3% - +3%
-0.37ms - +0.39ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
356.45ms - 365.31ms-unsure 🔍
-1% - +2%
-4.06ms - +6.71ms
unsure 🔍
-2% - +1%
-6.74ms - +4.18ms
tip-of-tree
tip-of-tree
356.49ms - 362.62msunsure 🔍
-2% - +1%
-6.71ms - +4.06ms
-unsure 🔍
-2% - +0%
-7.04ms - +1.82ms
previous-release
previous-release
358.97ms - 365.36msunsure 🔍
-1% - +2%
-4.18ms - +6.74ms
unsure 🔍
-1% - +2%
-1.82ms - +7.04ms
-
lit-html-template-heavy

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
72.05ms - 74.44ms-unsure 🔍
-5% - +1%
-3.45ms - +0.72ms
unsure 🔍
-4% - +1%
-2.99ms - +0.78ms
tip-of-tree
tip-of-tree
72.90ms - 76.31msunsure 🔍
-1% - +5%
-0.72ms - +3.45ms
-unsure 🔍
-3% - +3%
-1.99ms - +2.51ms
previous-release
previous-release
72.88ms - 75.81msunsure 🔍
-1% - +4%
-0.78ms - +2.99ms
unsure 🔍
-3% - +3%
-2.51ms - +1.99ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
166.57ms - 171.36ms-unsure 🔍
-2% - +2%
-3.14ms - +3.96ms
unsure 🔍
-2% - +3%
-2.53ms - +5.11ms
tip-of-tree
tip-of-tree
165.93ms - 171.17msunsure 🔍
-2% - +2%
-3.96ms - +3.14ms
-unsure 🔍
-2% - +3%
-3.09ms - +4.84ms
previous-release
previous-release
164.70ms - 170.65msunsure 🔍
-3% - +1%
-5.11ms - +2.53ms
unsure 🔍
-3% - +2%
-4.84ms - +3.09ms
-
reactive-element-list

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
80.29ms - 82.54ms-unsure 🔍
-2% - +2%
-1.24ms - +1.77ms
unsure 🔍
-2% - +2%
-1.37ms - +1.51ms
tip-of-tree
tip-of-tree
80.15ms - 82.15msunsure 🔍
-2% - +2%
-1.77ms - +1.24ms
-unsure 🔍
-2% - +1%
-1.54ms - +1.14ms
previous-release
previous-release
80.45ms - 82.24msunsure 🔍
-2% - +2%
-1.51ms - +1.37ms
unsure 🔍
-1% - +2%
-1.14ms - +1.54ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
1191.87ms - 1203.73ms-slower ❌
0% - 1%
0.49ms - 16.72ms
slower ❌
0% - 1%
2.18ms - 17.58ms
tip-of-tree
tip-of-tree
1183.65ms - 1194.73msfaster ✔
0% - 1%
0.49ms - 16.72ms
-unsure 🔍
-1% - +1%
-6.13ms - +8.68ms
previous-release
previous-release
1183.01ms - 1192.83msfaster ✔
0% - 1%
2.18ms - 17.58ms
unsure 🔍
-1% - +1%
-8.68ms - +6.13ms
-

update-reflect

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
1231.15ms - 1244.13ms-unsure 🔍
-1% - +1%
-7.63ms - +10.12ms
unsure 🔍
-0% - +1%
-4.58ms - +13.58ms
tip-of-tree
tip-of-tree
1230.35ms - 1242.45msunsure 🔍
-1% - +1%
-10.12ms - +7.63ms
-unsure 🔍
-0% - +1%
-5.52ms - +12.03ms
previous-release
previous-release
1226.79ms - 1239.49msunsure 🔍
-1% - +0%
-13.58ms - +4.58ms
unsure 🔍
-1% - +0%
-12.03ms - +5.52ms
-

tachometer-reporter-action v2 for Benchmarks

.changeset/fifty-monkeys-shop.md Outdated Show resolved Hide resolved
packages/reactive-element/src/css-tag.ts Outdated Show resolved Hide resolved
packages/lit-element/src/test/node-imports.ts Outdated Show resolved Hide resolved
Copy link
Member

@augustjk augustjk left a comment

Choose a reason for hiding this comment

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

Nice!

@aomarks aomarks merged commit 486739e into main Aug 18, 2022
@aomarks aomarks deleted the node-stylesheet branch August 18, 2022 00:38
@lit-robot lit-robot mentioned this pull request Aug 18, 2022
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.

[reactive-element] css tag function errors in Node build
2 participants