Skip to content
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

noahdarveau/buffer removal #2598

Merged
merged 13 commits into from
Nov 7, 2024
Merged

noahdarveau/buffer removal #2598

merged 13 commits into from
Nov 7, 2024

Conversation

noahdarveau-MSFT
Copy link
Contributor

@noahdarveau-MSFT noahdarveau-MSFT commented Nov 5, 2024

This PR replaces use of the Buffer package with uint8array-extras. This change allows for the removal of the buffer polyfill, which was previously required as Buffer was not supported in browsers. Since uint8array-extras is browser-compatible we do not need to include a polyfill for it. The Buffer polyfill is rather large, ~22KB when minified, and cannot reap the benefits of treeshaking since polyfills are always included in the global scope. We are only using the Buffer package in the clipboard capability, so even if a consumer was treeshaking out clipboard, they were still getting the Buffer polyfill included in their bundle, even though it was not doing anything. With this change, anyone not using the clipboard capability should see a flat ~22KB package size reduction.

To include this package, despite it being and NPM package, it was having issues working with Jest for our tests. I'm still not entirely sure what exactly the issue was, but copying the package locally and running it through our typescript transpiler fixed the issue. There was another issue with TextDecoder not being supported in jsdom, even though it is supported in both Node and browsers, so I included a jest-setup file that polyfills TextDecoder with the Node implementation.

@noahdarveau-MSFT noahdarveau-MSFT changed the title [DO NOT REVIEW] Testing PR for buffer removal noahdarveau/buffer removal Nov 5, 2024
@noahdarveau-MSFT noahdarveau-MSFT marked this pull request as ready for review November 5, 2024 22:38
@noahdarveau-MSFT noahdarveau-MSFT requested a review from a team as a code owner November 5, 2024 22:38
jadahiya-MSFT
jadahiya-MSFT previously approved these changes Nov 5, 2024
@noahdarveau-MSFT noahdarveau-MSFT enabled auto-merge (squash) November 6, 2024 00:34
Copy link
Contributor

@TrevorJoelHarris TrevorJoelHarris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of small questions and I want to follow up with jay about the transpiled uint8array-extras folder

@noahdarveau-MSFT noahdarveau-MSFT merged commit b783500 into main Nov 7, 2024
28 checks passed
@noahdarveau-MSFT noahdarveau-MSFT deleted the noahdarveau/treeshaking branch November 7, 2024 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants