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

Fix debug tests #3980

Open
wants to merge 8 commits into
base: v11-linked-list-prev-index-nextDom-2
Choose a base branch
from

Conversation

marvinhagemeister
Copy link
Member

This PR fixes the debug tests and a few compat ones. Got the total failure count down to 5. The remaining failures are all for forwardRef and it seems like we're putting ref back in props at different stages for vnodes vs internals. Haven't found out why though, but I think the existing fixes are already worth PR'ing.

@github-actions
Copy link

github-actions bot commented Apr 22, 2023

📊 Tachometer Benchmark Results

Summary

duration

  • 02_replace1k: 115.77ms - 118.76ms
  • 03_update10th1k_x16: 43.95ms - 44.73ms
  • 07_create10k: 1449.22ms - 1465.50ms
  • filter_list: 224.81ms - 231.03ms
  • hydrate1k: 84.02ms - 89.34ms
  • many_updates: 161.60ms - 164.99ms
  • text_update: 41.30ms - 42.44ms
  • todo: 49.02ms - 53.27ms

usedJSHeapSize

  • 02_replace1k: 6.70ms - 6.88ms
  • 03_update10th1k_x16: 3.30ms - 3.30ms
  • 07_create10k: 21.52ms - 21.55ms
  • filter_list: 1.69ms - 1.70ms
  • hydrate1k: 11.55ms - 12.39ms
  • many_updates: 4.41ms - 4.41ms
  • text_update: 1.07ms - 1.07ms
  • todo: 1.09ms - 1.10ms

Results

02_replace1k

duration

VersionAvg timevs
115.77ms - 118.76ms-

usedJSHeapSize

VersionAvg timevs
6.70ms - 6.88ms-

run-warmup-0

VersionAvg timevs
46.39ms - 48.46ms-

run-warmup-1

VersionAvg timevs
50.29ms - 53.51ms-

run-warmup-2

VersionAvg timevs
42.58ms - 44.07ms-

run-final

VersionAvg timevs
115.79ms - 118.78ms-
03_update10th1k_x16

duration

VersionAvg timevs
43.95ms - 44.73ms-

usedJSHeapSize

VersionAvg timevs
3.30ms - 3.30ms-
07_create10k

duration

VersionAvg timevs
1449.22ms - 1465.50ms-

usedJSHeapSize

VersionAvg timevs
21.52ms - 21.55ms-
filter_list

duration

VersionAvg timevs
224.81ms - 231.03ms-

usedJSHeapSize

VersionAvg timevs
1.69ms - 1.70ms-
hydrate1k

duration

VersionAvg timevs
84.02ms - 89.34ms-

usedJSHeapSize

VersionAvg timevs
11.55ms - 12.39ms-
many_updates

duration

VersionAvg timevs
161.60ms - 164.99ms-

usedJSHeapSize

VersionAvg timevs
4.41ms - 4.41ms-
text_update

duration

VersionAvg timevs
41.30ms - 42.44ms-

usedJSHeapSize

VersionAvg timevs
1.07ms - 1.07ms-
todo

duration

VersionAvg timevs
49.02ms - 53.27ms-

usedJSHeapSize

VersionAvg timevs
1.09ms - 1.10ms-

tachometer-reporter-action v2 for Benchmarks

@github-actions
Copy link

github-actions bot commented Apr 22, 2023

Size Change: +37 B (0%)

Total Size: 38 kB

Filename Size Change
compat/dist/compat.js 3.53 kB -4 B (0%)
compat/dist/compat.umd.js 3.6 kB -4 B (0%)
debug/dist/debug.js 3.12 kB +2 B (0%)
debug/dist/debug.umd.js 3.19 kB +5 B (0%)
dist/preact.js 4.63 kB +12 B (0%)
dist/preact.min.js 4.68 kB +14 B (0%)
dist/preact.umd.js 4.72 kB +12 B (0%)
ℹ️ View Unchanged
Filename Size Change
devtools/dist/devtools.js 232 B 0 B
devtools/dist/devtools.umd.js 317 B 0 B
hooks/dist/hooks.js 1.47 kB 0 B
hooks/dist/hooks.umd.js 1.55 kB 0 B
jsx-runtime/dist/jsxRuntime.js 296 B 0 B
jsx-runtime/dist/jsxRuntime.umd.js 379 B 0 B
server/dist/server.js 2.6 kB 0 B
server/dist/server.umd.js 2.69 kB 0 B
test-utils/dist/testUtils.js 437 B 0 B
test-utils/dist/testUtils.umd.js 522 B 0 B

compressed-size-action

Copy link
Member

@andrewiggins andrewiggins left a comment

Choose a reason for hiding this comment

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

Thanks for looking into this!

debug/src/validateMarkup.js Outdated Show resolved Hide resolved
vnode == null ||
vnode === true ||
vnode === false ||
typeof vnode === 'function'
Copy link
Member

Choose a reason for hiding this comment

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

Should we do this typeof once and reuse it's result here and line 249?

Same for patch

Copy link
Member Author

Choose a reason for hiding this comment

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

Just checked and storing the result in a variable is 10b larger. I think for now it's fine to leave it as is.

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