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

Rewrite node module handling (npm plugin) #874

Draft
wants to merge 41 commits into
base: main
Choose a base branch
from

Conversation

marvinhagemeister
Copy link
Member

@marvinhagemeister marvinhagemeister commented Sep 19, 2021

This PR is a rewrite of the way we handle node modules and fetch them from the npm registry.

  • Add support for browser field in package.json
  • Unify development and production handling into a single plugin. Previously, we used a middleware during development and a rollup plugin for production
  • Prebundles dependencies to greatly reduce the number of requests. Previously, we'd fetch each file of an npm package individually.
  • Improve CommonJS handling by attempting to convert it to ESM. Note that CommonJS in general has severe limitations in an ESM world and can't be served correctly.
  • Auto installing of npm packages is moved to a CLI flag --autoInstall and is disabled by default. The auto install feature is contained in a separate plugin.
  • Support for monorepo setups. We'll now follow the node resolution algorithm and traverse upwards until we find a node_modules folder with the package we're looking for.
  • npm registry is not a global anymore and can be set from cli via --registry.
  • Get's rid of the setCwd hack 🎉
  • We didn't have any tests regarding the npm plugin, so this PR adds them. They make up the lion share of the changes.

Todos:

  • Support non-js files from npm packages
  • Bring back package versioning (only for auto install)
  • Fully remove previous npm plugin
  • Bring back etag caching
  • Bring back brotli compression background upgrades - already present

Fixes #299, fixes #784, fixes #285, fixes #135, fixes #109, fixes #151, fixes #167

@changeset-bot
Copy link

changeset-bot bot commented Sep 19, 2021

🦋 Changeset detected

Latest commit: 3043967

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
wmr Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@marvinhagemeister marvinhagemeister force-pushed the npm-refactor2 branch 2 times, most recently from 87b2ca9 to 3a5a48c Compare October 6, 2021 20:10
log(kl.dim(`downloading... `) + kl.cyan(id));
// Download tarball to disk
const tarPath = path.join(downloadDir, `${safeName}-${version}.tgz`);
await streamToDisk(tarball, tarPath);
Copy link
Member

Choose a reason for hiding this comment

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

seems like this could instead stream directly into parseTarball, to skip the disk write+read and cut memory usage in half.

@bakeiro
Copy link

bakeiro commented Jun 1, 2022

Hey!

Nice PR, I need this change in order to work with wmr... maybe I can help somehow to make this PR go through?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants