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

--source pika #228

Merged
merged 6 commits into from
Mar 4, 2020
Merged

--source pika #228

merged 6 commits into from
Mar 4, 2020

Conversation

FredKSchott
Copy link
Owner

@FredKSchott FredKSchott commented Feb 26, 2020

Original Discussion / PRD: #205
This PR is now ready for review.

Background

Snowpack is a post-install tool. It runs after npm install, and it essentially exists to convert your npm packages (in your "node_modules/" directory) into JS files that run in the browser without a bundler (written to a "web_modules/" directory). Because of this flow, Snowpack has an implicit dependency on npm/yarn and a direct dependency on Rollup, Babel, etc.

Pika CDN is a hosted CDN of npm packages. Whenever an npm package is requested, it is built into a JS file that run in the browser without a bundler (served in the response). Request a package, and get back the ESM code of that package.

Investigation

The question that I've been exploring over the last couple of weeks: If Pika CDN is already a hosted source of ESM package code, can we just pull code from the CDN instead of installing/building locally every time we run Snowpack?

The pros here are obvious: Instead of running npm/yarn and THEN running Snowpack, you just run Snowpack. Snowpack can manage your frontend dependencies for you.

Implementation

  • If --source pika: Run a pre-pass step to resolve each webDependency on the Pika CDN. If it succeeds (dependency exists on the CDN) store the response in a local cache to use later.
  • On install, check our local cache for CDN-resolved dependencies. If found, read it from there and resolve any sub-dependencies through the CDN as well. Otherwise, fall-back to a lookup in your local node_modules directory.
  • On finish, write a "lockfile" (import map) to the current working directory. On future installs, Snowpack will read the URLs from this lockfile instead of re-resolving them and potentially pulling in new dependencies.

Documentation

Screen Shot 2020-02-26 at 9 53 12 AM

/cc @DangoDev @alex-saunders @monchi @stramel
@NotWoods you might like this as well, given our conversation the other week :)

@@ -18,7 +18,7 @@
"publish": "pika publish",
"version": "npm run build",
"test": "npm run test:integration && npm run test:babel-plugin",
"test:integration": "jest test/integration/runner.js --test-timeout=15000 --testMatch \"**/test/**/*.[jt]s?(x)\"",
"test:integration": "jest test/integration/runner.js --test-timeout=15000 --testMatch \"**/test/**/*.[jt]s?(x)\" -i",
Copy link
Owner Author

Choose a reason for hiding this comment

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

-i sped up our tests a bit, and will guarantee that tests run serially vs. in parallel (this will become more important now that we have a local cache to worry about)

@FredKSchott FredKSchott force-pushed the source-pika branch 2 times, most recently from 49d001b to 495226d Compare February 26, 2020 18:10
@FredKSchott FredKSchott mentioned this pull request Feb 26, 2020

Snowpack defaults to reading packages from your local `node_modules/` directory, but you can control this behavior with the `--source` flag to simplify your Snowpack install step even further.

Use `--source pika` to have Snowpack fetch packages directly from the [Pika CDN](https://cdn.pika.dev/) instead of your local `node_modules/` directory. Your app will still get the same npm package code, but now Snowpack is fully installing and managing your npm dependencies for you. **With `--source pika` you no longer have to run `npm install` before running Snowpack.**
Copy link
Collaborator

@drwpow drwpow Mar 2, 2020

Choose a reason for hiding this comment

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

Does the Pika CDN have any other benefits to using this? Or is the main benefit speed?

At least in my tests, I noticed this to be pretty snappy, and the caching seemed to work just fine. I’m wondering if it’s worth calling more attention to the faster Snowpack build times here. Or if there’s some other benefit I’m not thinking of (my brain went to some of the Pika CDN browser-specific stuff, but I think that doesn’t apply for build-time Snowpack, right?).

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yea, speed and simplicity are the two main benefits here. Pika CDN can do a ton of cool stuff when you point your site to it, but in this context, we're just using it to get the modern JS code and then pass it through Snowpack's install (build) step.

@drwpow
Copy link
Collaborator

drwpow commented Mar 2, 2020

This works great! And in a sample project it was pretty fast (and basically instant on subsequent re-runs 🏎️)

One potential cache thing: earlier I reported a bug where it didn’t finish installing (because of a Pika CDN thing? or something). But I think snowpack.lock.json was still created and something still got cached even though it failed. As a result, when I tried again, the install failed.

However, clearing the cache (blowing away node_modules) worked and it ran first try. I’m not sure by staring at this code, but if any part of the install fails, does any part of it get cached? Should we blow away the cache on error? It could possibly prevent some weirdness.

}
if (packageSemver.startsWith('npm:@reactesm') || packageSemver.startsWith('npm:@pika/react')) {
throw new Error(
`React workarounds no longer needed in --source=pika mode. Revert to the official React & React-DOM packages.`,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it worth also mentioning this in the docs? Probably with the react guide?

Copy link
Owner Author

Choose a reason for hiding this comment

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

great call

@FredKSchott
Copy link
Owner Author

@DangoDev I think when you last ran it we didn't protect the cache from bad responses as well as we do now. Now that should be impossible, but I can confirm.

It would be great to have a self-healing cache, but for our own sanity as contributors we should add a --reload flag to clear the cache before running.

@FredKSchott
Copy link
Owner Author

Thanks everyone for reviewing! I added --reload support to bust a broken cache, and added an "experimental" warning with --source pika so that we can merge and continue to experiment before locking in support.

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