fix(price-service-sdk): make price-service-sdk portable #2242
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
cprussin
force-pushed
the
cprussin/make-price-service-sdk-browser-compatible
branch
2 times, most recently
from
d46cac4 to
8c0f107
Compare
tejasbadadare
approved these changes
Jan 10, 2025
The price-service-sdk package previously used `Buffers` in it's interface in many places. However, `Buffer` is a node-specific interface which does not exist in browsers. Many browsers ship [a polyfill](https://github.com/feross/buffer), however I've discovered that the polyfill is not actually fully compatible with the Node module. In particular, while the Node module [supports aliases such as `readUint8` for `readUInt8`](https://nodejs.org/api/buffer.html#bufreaduint8offset) (note the difference in capitalization), [the polyfill does not](https://github.com/feross/buffer/blob/master/index.d.ts). As a result, attempting to utilize `price-service-sdk` in the browser was either going to cause errors because the Buffer sdk isn't present, or it would cause errors because of the use of aliased function names. There were three options to fix this issue: 1. Switch everything to using unaliased function names. This would work portably, but only assuming that the Buffer polyfill is present. 2. Switch everything to using [`Uint8Array`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Uint8Array), which is a more generic [typed array](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray) and largely supersedes the need to use the `Buffer` class at all. This would work, however it would be a breaking change. 3. Switch everything to using a generic interface which accepts anything which extends `Uint8Array`. This requires a few gross typescript hacks to type check correctly and avoid anything being a breaking change, but it makes the code the most portable without requiring any major version or any consumer code changes. This commit implements option 3.
cprussin
force-pushed
the
cprussin/make-price-service-sdk-browser-compatible
branch
from
8c0f107 to
7727d3c
Compare
|
thanks @tejasbadadare , I updated the version |
guibescos
reviewed
Jan 13, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The price-service-sdk package previously used
Buffersin it's interface in many places. However,Bufferis a node-specific interface which does not exist in browsers.Many browsers ship a polyfill, however I've discovered that the polyfill is not actually fully compatible with the Node module. In particular, while the Node module supports aliases such as
readUint8forreadUInt8(note the difference in capitalization), the polyfill does not.As a result, attempting to utilize
price-service-sdkin the browser was either going to cause errors because the Buffer polyfill isn't present, or it would cause errors because of the use of aliased function names.There were three options to fix this issue:
Switch everything to using unaliased function names. This would work portably, but only assuming that the Buffer polyfill is present.
Switch everything to using
Uint8Array, which is a more generic typed array and largely supersedes the need to use theBufferclass at all (and which is a superclass ofBuffer). This would work, however it would be a breaking change.Switch everything to using a generic interface which accepts anything which extends
Uint8Array(includingBuffer), and useUint8Arraymethods exclusively insideprice-service-sdk. This requires a few gross typescript hacks to type check correctly and avoid anything being a breaking change, but it makes the code the most portable without requiring any polyfills or requiring a major version bump.This commit implements option 3.