Modularize compat src and tests#2124
Conversation
| // ...but only on the first render, see #1828 | ||
| if (parent._children == null) { | ||
| while (parent.firstChild) { | ||
| parent.removeChild(parent.firstChild); |
There was a problem hiding this comment.
Most of the byte savings come from removing the call to preact/src/util.removeNode here and just calling removeChild directly since we are looping through the parent element anyway: b1443c5
| import { assign, removeNode } from '../../src/util'; | ||
| import { SuspenseList } from './suspense-list'; | ||
| import { createPortal } from './portals'; | ||
| import { render, REACT_ELEMENT_TYPE } from './render'; |
There was a problem hiding this comment.
Fiddling with the order of the array imports led to some byte savings gains here
|
Forgot to mention that I'm exploring ways to make "preact/compat" more tree-shakeable and so another motivation for this change is to better isolate which parts of the code require which |
| import { assign } from './util'; | ||
|
|
||
| let oldVNodeHook = options.vnode; | ||
| options.vnode = vnode => { |
There was a problem hiding this comment.
wondering if we should maybe keep these "side-effects" in a central place, maybe entrypoint so they don't bite us at some point
There was a problem hiding this comment.
In my next PR I was actually playing with ways to only install options when a consumer needs them, to try and make preact/compat more tree-shakeable. This works well if each module contains its own option hook so it's easy to know what code is needed for that feature to work.
Though it does have a downside of potentially leading to duplication of options, but I think if we can come up with a pattern that enables tree-shaking, it might be worth it.
There was a problem hiding this comment.
Hmm, maybe putting them in the same files should be in that PR then, if you can figure out a way to do it 🤷♀️
marvinhagemeister
left a comment
There was a problem hiding this comment.
I love this! It's much easier to navigate the source now. Fantastic work Andre 🙌 💯
cristianbote
left a comment
There was a problem hiding this comment.
Awesome job @andrewiggins! 🎉
compat/src/index.jsstarted to get pretty big which I think was hindering good contributions to it. While trying to understand it, I discovered lots of disparate repetitions while working on this PR (those are captured in #2116).My goal with this PR is to make knowing where a change should go in compat a little easier by breaking up index.js into separate modules, hopefully reducing future duplication and making it easier to change compat.
I also did the same exercise with the compat tests, cuz I found a good amount of duplication there too (and in one case that one copy of a test had been updated but not the other lol).
Total change:
compat.js.gz: -12 B