search

davidmyersdev / vite-plugin-node-polyfills

A Vite plugin to polyfill Node's Core Modules for browser environments.
MIT License
263 stars 17 forks source link

patch `buffer` to enforce global `Uint8Array` is used #40

Closed atlowChemi closed 10 months ago

atlowChemi commented 10 months ago

I would like to take the chance to mention I really appreciate this awesome plugin, Thanks for your work!

Fixes: https://github.com/davidmyersdev/vite-plugin-node-polyfills/issues/36

atlowChemi commented 10 months ago

I am not quite sure what would be the way to test this, but basically this makes sure the globals used are the actual global values

davidmyersdev commented 10 months ago

@atlowChemi thanks so much for the PR! I'm not sure what the implications of using a patched dependency might be on downstream consumers of this library, so I'll have to test it against a few of the test apps that people have submitted for other scenarios. As soon as I can confirm that this works as is, I'll merge it. 😁

atlowChemi commented 10 months ago

I'm not sure what the implications of using a patched dependency might be on downstream consumers of this library

What implications are you concerned of? 🙂 From my playing around with it, these changes reflected in the shims/banner/dist/index.cjs, and my build seemed to pass after that 🙏🏽

adrian-gierakowski commented 10 months ago

Seems like this would only work for pnpm users, no?

atlowChemi commented 10 months ago

@adrian-gierakowski nope, this affects the buffer package being installed when working on the vite-plugin-node-polyfills package. The vite-plugin-node-polyfills is then getting bundled and shipped with the modified buffer package, which means the consumers can use any package manager and still get the "fixed" version...

davidmyersdev commented 10 months ago

I have confirmed that this works as expected without any side effects that I could find. Thanks again for the PR!