fix: Mark module as not containing side effectful code#141
Conversation
| "./prerender": "./src/prerender.js", | ||
| "./hydrate": "./src/hydrate.js" | ||
| }, | ||
| "sideEffects": false, |
There was a problem hiding this comment.
I am not sure this is safe everywhere, we might need to do a if (!initialized) registerOptionsHooks() in the relevant components
There was a problem hiding this comment.
How so?
The only issue I can see is if the user is using the <Router> as an implicit suspense boundary without ever using lazy() (or ErrorBoundary) -- this is a very unlikely scenario, given suspense's low usage, nor do I think it's one we should support by injecting more code than actually asked for or needed.
As for normal usage, every modern bundler will see the side-effectful usage within individual modules and keep the hooks around if the module as a whole is ever loaded, i.e., if the user ever imports lazy or ErrorBoundary. I don't know how non-modern bundlers act here but I don't think we need to be preemptively concerned.
I don't foresee anything else being problematic here, but perhaps I'm missing something?
Reported here: https://bsky.app/profile/sdhfjksdf.bsky.social/post/3mi526vu74224
Can reproduce by creating a new preact project (
pn create preact) with only prerendering enabled. Our option hook usage is side-effectful, but is fairly safe to shake out when the user isn't consuminglazyor<ErrorBoundary>.For the example case above, in which the user is only using
hydrate(and technicallyprerender, but that's split into a separate bundle entirely), the bundle size changes as such:I spent a while playing with this and I couldn't get it to break with Vite. This seems to only shake at the sub-file level, which is precisely what we want, so that the
_diff&_catchErroraugments (for example) are kept when usinglazyorErrorBoundary. Everything from that file will be shaken if none of its exports are used, as they should be.