fix(isBuffer): add browser export condition to avoid 44KB Buffer polyfill#1671
fix(isBuffer): add browser export condition to avoid 44KB Buffer polyfill#1671
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Adds a browser-safe build path for isBuffer to prevent bundlers (e.g. webpack/Next.js, Vite) from injecting large Buffer polyfills, and refactors internal call sites to use the centralized predicate.
Changes:
- Introduces
isBuffer.browser.tsstub and a Rollup alias to swap it into the browser ESM/UMD builds. - Adds
browserexport conditions for top-level subpath exports inpublishConfig.exports. - Replaces inline
Bufferchecks in several utilities (isEqualWith,cloneDeepWith,isEmpty,mergeWith) withisBuffer().
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/predicate/isEqualWith.ts | Refactors Buffer detection in typed-array comparison to use isBuffer(). |
| src/predicate/isBuffer.browser.ts | Adds browser-safe isBuffer stub used by the browser build. |
| src/object/cloneDeepWith.ts | Switches Buffer cloning branch to use centralized isBuffer(). |
| src/compat/predicate/isEmpty.ts | Replaces inline Buffer check with isBuffer() in compat isEmpty. |
| src/compat/object/mergeWith.ts | Uses isBuffer() when deciding whether to deep-clone Buffer source values. |
| rollup.config.mjs | Adds browser ESM build output + alias plugin wiring for isBuffer stub. |
| package.json | Adds browser conditional export mappings under publishConfig.exports. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Hello, thanks for your pull request! Your change definitely fixes the bundle size bloat. That said, I'm a bit concerned that having multiple builds could lead to larger install sizes over time, and it would make adding additional subpath exports more complex down the line. As an alternative, I took a different approach and used |
|
when fixing this bug claude also suggested the to be more future proof and to ensure it works with every bundler I removed the Buffer part entirely so a bundle update won't silently add this bug again also targeted builds are always smaller than isomorphic builds but I can totally understand that you don't want to maintain the more complex build setup I will try the globalThis hack in webpack turbopack and tanstack start and let you know - rspack2 is hard to test right now as it is still alpha |
…k/turbopack polyfill Webpack and Turbopack auto-inject the ~20KB `buffer` polyfill into browser bundles whenever they see the free identifier `Buffer`, even when every call site is guarded by `typeof Buffer !== 'undefined'` — the guard runs too late, the bundler has already decided. Rewriting the 5 call sites (`isBuffer`, `isEqualWith`, `cloneDeepWith`, compat `mergeWith`, compat `isEmpty`) to go through `globalThis.Buffer` turns the bare identifier into a property access that neither plugin triggers on. Node behaviour is unchanged. Alternative to toss#1671 (no rollup alias plugin, no browser export condition, no second build path). Measured in a minimal Next.js 16 app importing cloneDeep client-side: - webpack: -21.97 KB raw / -6.68 KB gz - turbopack: -22.12 KB raw / -6.69 KB gz - vite: ±0 KB (already rewrites bare Buffer to globalThis.Buffer itself) closes toss#1670
|
hey @raon0211 tested the globalThis.Buffer route, tested it across 3 bundlers just realised you already changed it on this branch numbers on a minimal next 16 + tanstack start (vite 8) app that imports cloneDeep client-side:
turbopack number is smaller than what I got here originally (-44 KB) it already rewrites bare Buffer → globalThis.Buffer itself, so no polyfill ever got injected there. sanity check on the next/webpack client chunk: pages/index-*.js drops from 26 KB → 3.7 KB, and _isBuffer / #1691 is just the globalThis change |
|
I also measured your approach is |
Yup, thanks for pointing that out. I think that's an acceptable tradeoff to keep the overall library complexity down. |
|
@raon0211 thanks for releasing! |
isBufferstub (() => false) and a rollup alias plugin that swaps it in during the browser buildbrowserexport condition to every subpath inpackage.jsonso bundlers like webpack/vite pick up the polyfill-free build automaticallytypeof Buffer/Buffer.isBuffer()checks acrossmergeWith,isEmpty,cloneDeepWith, andisEqualWithto use the centralizedisBufferfunctioncloses #1670
Build size comparison (Next.js test app)
Webpack
Turbopack
Test plan
yarn vitest run— all existing tests pass (isBuffertests run against the real Node.js implementation)yarn lint/tsc --noEmit— clean.next/staticdrops from 492K to 464K after the changegrep -r "Buffer" dist/browser/confirms no Buffer references in the browser build