-
Notifications
You must be signed in to change notification settings - Fork 301
fix(lazer-sdk): implemented function to convert all non-string WS frames to an array buffer, then leverage an isomorphic buffer implementation #3139
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good! I have minor nits / points to consider but nothing that I would block merging on, feel free to consider / address any or disregard as you see fit.
update: given the branch on which this PR was crafted is fairly old, I'm not confident in my knowledge of the changeset context (it's my 4th day, after all 😂 ), I'll be taking some of the feedback given on this PR and crafting the changes on the latest code available on |
…lso add Buffer support when running in the browser (plus better runtime env detection)
…e being sent on node WebSocket connections
f15c41b
to
84190a4
Compare
@cprussin @tejasbadadare would appreciate another pass over this, as I've updated this branch and implement changes with, hopefully, a bit better organization (like the new Responded to most PR feedback left in the prior iteration 🤞 |
@cprussin @tejasbadadare responded to all PR feedback. I'm going to let this simmer until you all log on for the day, as I haven't released an NPM package here yet and want to be sure I do it in a manner that you all are expecting. I see the Readme has steps outlined, so I'll run with those and ping if I have questions |
requires a re-review from you all (thanks for your patience here) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent!
Partially-resolves PFI-101
Summary
As the title suggests, there was a bug in the lazer Websocket client, which was built and intended to built use isomorphically across browser and server-side JavaScript environments.
However, the current implementation made assumptions that the
Buffer
class would be available in all environments, when it is not something that exists in the browser.As such, this PR adds the following:
ArrayBuffer
IsomorphicBuffer.from()
functionThis allows all existing logic to remain in place, minimizing changes or needing to create approximations of functions but against the
ArrayBuffer
class.Rationale
This feels like the least risky way to fix the issue with isomorphism. This PR does not address the auth token being provided in the
QueryString
at WSS open time. This will occur in another PR, as that will have some business logic changes that change how connections are created and destroyed.How has this been tested?
see videos, below
Working in Node
CleanShot.2025-10-15.at.16.12.29.mp4
Working in the browser
CleanShot.2025-10-15.at.16.40.22.mp4