Repository navigation
Conversation
semver ships in the manager bundle, which runs in the browser. slimsemver is a drop-in replacement that tree-shakes, so only the functions core actually uses get bundled. Also drops the @types/semver devDependency, since slimsemver ships its own types.
|
Damin3927
changed the title
There was a problem hiding this comment.
Pull request overview
This PR replaces semver with slimsemver throughout code/core to reduce how much semver-related code ends up in the browser/manager bundle by switching to an ESM, tree-shakeable implementation.
Changes:
- Swap
semverimports toslimsemveracross core (default and named-function import styles). - Update deep function imports (e.g.
functions/sort.js) to point atslimsemver. - Replace the
semverdependency withslimsemverincode/core/package.jsonand remove@types/semver.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| code/core/src/telemetry/get-known-packages.ts | Switches telemetry semver parsing/coercion to slimsemver. |
| code/core/src/manager-api/modules/versions.ts | Switches manager version comparisons/URL versioning logic to slimsemver. |
| code/core/src/core-server/utils/update-check.ts | Switches server-side update-check semver comparisons to slimsemver. |
| code/core/src/common/node-version.ts | Switches Node version range checks to slimsemver. |
| code/core/src/common/js-package-manager/util.ts | Switches package manager semver helpers to slimsemver. |
| code/core/src/common/js-package-manager/PNPMProxy.ts | Switches pnpm version gating/coercion to slimsemver. |
| code/core/src/common/js-package-manager/NPMProxy.ts | Switches deep sort import to slimsemver/functions/sort.js. |
| code/core/src/common/js-package-manager/JsPackageManager.ts | Switches shared package manager semver logic to slimsemver. |
| code/core/src/common/js-package-manager/BUNProxy.ts | Switches deep sort import to slimsemver/functions/sort.js. |
| code/core/src/cli/helpers.ts | Switches CLI semver coercion/range checks to slimsemver. |
| code/core/src/cli/detectLanguage.ts | Switches language detection’s semver checks to slimsemver. |
| code/core/src/cli/AddonVitestService.ts | Switches addon-vitest compatibility semver utilities to slimsemver. |
| code/core/package.json | Replaces semver with slimsemver dependency; removes @types/semver. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import memoize from 'memoizerific'; | ||
| import semver from 'semver'; | ||
| import semver from 'slimsemver'; | ||
|
|
| import picocolors from 'picocolors'; | ||
| import semver from 'semver'; | ||
| import semver from 'slimsemver'; | ||
| import { dedent } from 'ts-dedent'; |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
WalkthroughChangessemver dependency replacement
Possibly related PRs
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Correcting myself: the number in this PR is wrong, and the change isn't worth making. Closing it. I measured the module closure gzipped, which is not what a bundler emits. Running it through esbuild the way your build actually would:
So the real saving is 1.9 KB gzip, not 19 KB — about 0.5% of the manager bundle, not 5%. The premise was wrong too. I claimed 1.9 KB is not a reason to take on a new dependency, least of all one published this week. Sorry for the noise, and thanks to whoever's triage queue this landed in. |
What I did
Swapped
semverforslimsemverincode/core, and dropped the@types/semverdevDependency since slimsemver ships its own types.code/corepulls in all ofsemver, and part of that ends up in the manager bundle, which runs in the browser —dist/manager/globals-runtime.jscontains semver's error strings, so the whole library is in there.Core imports
coerce,diff,gt,gte,major,minor,minVersion,patch,prerelease,satisfies,sort,validandvalidRange.semveris CommonJS with noexportsfield, so a bundler can't drop the rest.slimsemver is an API-compatible reimplementation that ships ESM and tree-shakes. For the set core uses that's 5.1 KB gzip instead of 24.6 KB — roughly 19 KB off a manager bundle that is currently ~373 KB gzip.
Only the import specifier and the dependency change; no call sites were touched.
Two things you should weigh:
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
The existing
code/coreunit tests cover the call sites, since nothing but the import specifier changed.On the library itself: it runs a differential suite against
semveron every commit — 119,796 cases comparing return values and thrown error types across the range grammar and all option combinations — plus a seeded fuzzer. I also ran core's exact call sites against both libraries side by side (301 checks, identical output). Happy to share that script if it's useful.Manual testing
The browser-side usage is the version check in
manager-api, so that's what's worth exercising:yarn task --task sandbox --start-from auto --template react-vite/default-tsIf it's easier, temporarily setting
versions.latestto something higher than the local version incode/core/src/manager-api/modules/versions.tswill force the upgrade path.I haven't run the monorepo build locally, so CI is the real check here.
Documentation
No user-facing behaviour changes, so no docs needed.