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

Make Preact on skypack optional? #106

Open
eyelidlessness opened this issue Jan 27, 2021 · 8 comments
Open

Make Preact on skypack optional? #106

eyelidlessness opened this issue Jan 27, 2021 · 8 comments
Labels
enhancement New feature or request
Milestone

Comments

@eyelidlessness
Copy link
Collaborator

Would you consider making the usage of Skypack CDN URLs for Preact optional? This could potentially be accomplished by removing the mapping logic and deferring to the built in behavior introduced in Snowpack 3.0.

My motivation is that using CDNs for external dependencies is actually increasingly a performance negative, since most browsers now implement cache partitioning.

I spent some time trying to make this change on my fork but it turned out to be fairly involved, and I wanted to check in about whether it would be a welcome change before I get deeper into it.

@natemoo-re
Copy link
Owner

Yep, I'm good with this change.

I'm not sure if the built-in Skypack CDN behavior will work with Microsite's SSR setup... might be better to just support local builds for now.

@eyelidlessness
Copy link
Collaborator Author

I’ve thought about ways it could work (also tying in with #99), but after so many boondoggles around tests I’m definitely happy to keep it as small and focused as possible for a first start. I doubt I’ll look at it tomorrow, I’m going to yeet myself out to space if I don’t have a meaningful commit on my own site tomorrow. But I’ll be back to this, I definitely want it.

@eyelidlessness
Copy link
Collaborator Author

One more note on this because it’s something I’m tracking on my site: I tried a bunch of stuff locally with the contexts used to gather head/hydration to preload as much as possible and it seems Skypack itself is a bottleneck here. Going local already felt like a good direction given cache partitioning, but to add more flavor here, the Skypack CDN has a pretty much fixed 200-300ms latency and their cache headers don’t do well with even non-force reloads.

@natemoo-re
Copy link
Owner

@eyelidlessness Makes sense. I'm taking a look at this now and I'll see where I get.

@natemoo-re
Copy link
Owner

OK, you warned me but this was much more involved than I expected. 😅

What the build script does right now is mark preact as external for the SSG pass, which allows node to resolve preact to the same instance that microsite is using from node_modules. As a final pass Microsite transforms any references to bare preact in the generated dist files to point at the CDN. This works smoothly because we're only dealing with a single instance of preact, whereas if we include preact in the bundle, we're dealing with a version local to the user's components and a version local to Microsite's wrapper components. Due to Preact's architecture, despite both instances being the same exact version of Preact, they can't be mixed.

There's definitely a way around this, but I think it requires some more thought. I have a feeling v1.2.0 is going to be focused on eliminating the differences between the dev/build scripts, so this should fit nicely into that set of refactoring.

@natemoo-re natemoo-re added this to the v1.2.0 milestone Jan 29, 2021
@natemoo-re natemoo-re added the enhancement New feature or request label Jan 29, 2021
@eyelidlessness
Copy link
Collaborator Author

I’m still in early caffeination stage but this feels like it might be an easy solve by making Preact a dev/peer dependency? That way for users there should only be one version and no mixing

@natemoo-re
Copy link
Owner

Unfortunately not, Snowpack bundles Preact into <STAGING_DIR>/_snowpack/pkg/preact.js, so userland components all pull that Preact instance while the Microsite SSG pass pulls from node_modules. It shouldn't be too hard to solve, just want to prioritize getting v1.1.0 out the door finally.

@natemoo-re
Copy link
Owner

Just cross-referencing the fact that in #125, I promised Preact on Skypack would opt-in behavior in v1.2.0.

@Siilwyn I agree with that! Local will always be better for performance.

  • v1.1.0 will remain on Skypack, purely because there are too many unrelated fixes/features that have needed to go out for a while.
  • v1.2.0 will load Preact from a local vendor chunk by default. I can start on the architectural changes ASAP. Would love your help @eyelidlessness!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants