-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Optimizations 1 #3377
base: v11
Are you sure you want to change the base?
Optimizations 1 #3377
Conversation
…this` to access the associated Internal.
… of their Internals.
… components (forceUpdate, setState, FC's, hooks, etc). Anything with a reference to an Internal can call `render(callback)` on that Internal.
📊 Tachometer Benchmark ResultsSummaryduration
usedJSHeapSize
Results02_replace1k
duration
usedJSHeapSize
run-warmup-0
run-warmup-1
run-warmup-2
run-warmup-3
run-warmup-4
run-final
03_update10th1k_x16
duration
usedJSHeapSize
07_create10k
duration
usedJSHeapSize
filter_list
duration
usedJSHeapSize
hydrate1k
duration
usedJSHeapSize
many_updates
duration
usedJSHeapSize
text_update
duration
usedJSHeapSize
todo
duration
usedJSHeapSize
|
Size Change: -334 B (1%) Total Size: 32.4 kB
ℹ️ View Unchanged
|
@@ -34,6 +34,7 @@ | |||
"props": { | |||
"cname": 6, | |||
"props": { | |||
"$flags": "f", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would opt to keep this to flags
to encourage use of library authors for additional extensions that we don't want to support first-party
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been going back and forth on it. It pains me that this is 23b just for lags
👁
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, the community being able to use this without looking at this file kind off boats well for me but yeah, one opinion 😅 also the types won't ever reflect .f unless we start doing two different type files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
true about types too (though that makes me think we should probably already be doing internal VS external files given we have other minified properties)
src/component.js
Outdated
commitRoot(commitQueue, internal); | ||
} | ||
} | ||
const renderDirtyInternal = internal => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's maybe rename this to renderInternal :p
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
renderEnqueuedInternal?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes something like that, it's a nit, the dirty part feels a bit odd 😅
} | ||
// re-render subscribers in response to value change | ||
else if (props.value !== this._prev) { | ||
this._subs.forEach(enqueueRender); | ||
this._subs.forEach(i => i.render()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this._subs.forEach(i => i.render()); | |
this._subs.forEach(i => { i.render() }); |
Saves some bytes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ha! can't believe I missed that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JoviDeCroock would you believe that actually adds 3b?! gzip
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it comes from the return
keyword added at build time 🤷
internal.rerender()
tointernal.render()
(gzip optimizations, since "render" already appears in source)enqueueRender()
into the implementation ofinternal.render()
this
to access the Internal instance to be enqueued.enqueueRender()
is now instead callinginternal.render()
, which removes the direct dependency on that function and improves tree-shakingcreateRoot()
render()
andhydrate()
exported functionsrerenderQueue
-->renderQueue
(not always repeated renders, also I type "rerender" wrong every time)process()
-->processRenderQueue()
createVNode()
and usecreateElement()
everywhere instead.flags
to.f
(-23b)