mtth
global Buffer usage complicates browser usage
Accessing Buffer as a global works great in Node.js, but in the browser requires each build tool to inject that global in its own special way.
Would you be open to a small PR that uses import/require to access Buffer to simplify browser build issues?
I was also facing this issue. I would like you to fix this as soon as possible.
Uncaught ReferenceError: Buffer is not defined
at newBuffer (utils.js:20:1)
at new BufferPool (utils.js:311:1)
at ./node_modules/avsc/lib/utils.js (utils.js:12:1)
at options.factory (react refresh:6:1)
at __webpack_require__ (bootstrap:24:1)
at fn (hot module replacement:62:1)
at ./node_modules/avsc/lib/types.js (types.js:14:1)
at options.factory (react refresh:6:1)
at __webpack_require__ (bootstrap:24:1)
at fn (hot module replacement:62:1)
Hi @JAForbes. Thanks for reporting and apologies for the slow response. I'm not familiar with the underlying issue: could you help me understand how complex browser setup is currently and what a fix would look like?
Hey @mtth, no worries at all!
This library uses Buffer without importing it, which works in Node because buffer happens to be a global. But in other JS contexts, Buffer is not a global because it is Node specific.
Some bundlers will detect its usage and inject a polyfill automatically, I believe Webpack does this. Other bundlers are more spec compliant and do not infer that a variable named Buffer necessarily is the Node.js global. But if you use import or require they can know you are referring to the Node.js built in and suggest the user adds a polyfill.
If users are using avsc with vite or rollup, they need to manually add the polyfill and attach it to the window to prevent avsc from crashing. But if thlis library simply imported Buffer instead of relying on a global, their build would fail with a helpful warning suggesting they install rollup-plugin-node-polyfills or similar.
A longer term fix would be to use ArrayBuffer which is available in Node / Deno / Browser / Workers, but a short term fix would be to just ensure any files that use Buffer explicitly import it. I'd be happy to throw up a PR it should only be a few files changed with 1 new LOC.
That's helpful, thank you. This change sounds reasonable to me, please put together the PR when you have a chance.
W.r.t. ArrayBuffer, if I remember correctly, Buffer provided additional/optimized methods so a switch might not be performance neutral (at least in older Node versions). This might be the case for newer versions though.