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

feat: support UInt8Array in place of Buffer #1083

Merged
merged 2 commits into from
Jan 9, 2025

Conversation

kylecarbs
Copy link
Contributor

@kylecarbs kylecarbs commented Dec 31, 2024

This seems reasonable, considering memfs supports the browser, and users are more likely to have instances of Uint8Array there.

Resolves #733

@kylecarbs kylecarbs changed the title fix: allow Uint8Array in pathToFilename fix: allow UInt8Array in pathToFilename Dec 31, 2024
@G-Rath
Copy link
Collaborator

G-Rath commented Dec 31, 2024

I feel like this probably is a fix for #733 - would you mind double checking that for me, and throwing in a test or two as/when you think is needed? (if you don't think it's worth having any, I'm ok with that too 🙂)

@kylecarbs
Copy link
Contributor Author

@G-Rath seems to fix #733! I added tests before, but they don't do much - the structure is the same for Uint8Array. I'm happy to add it if you'd prefer, though!

@G-Rath
Copy link
Collaborator

G-Rath commented Jan 3, 2025

Awesome! Mind sticking a "resolves" line to your description?

Re tests it doesn't surprise me but would still be good if we had at least one where were explicitly passing in a Uint8Array

@kylecarbs
Copy link
Contributor Author

@G-Rath added to the description! I can add one before merge - no biggie.

@kylecarbs
Copy link
Contributor Author

@G-Rath fixed. I also removed a debugger; line that was causing me some issues, just assuming that's fine.

@kylecarbs
Copy link
Contributor Author

Seems like there are some unrelated flakes failing - but test I added should be good.

This seems reasonable, considering memfs supports the browser,
and users are more likely to have instances of Uint8Array there.
@G-Rath
Copy link
Collaborator

G-Rath commented Jan 9, 2025

I also removed a debugger;

Yeah that should never have been landed - I've opened #1086 to remove it and another from a test, just so we've got a dedicated changelog entry as it's a whole bug of its own

Seems like there are some unrelated flakes failing

A couple of the watch ones are very slightly flakey, but thankfully only slightly so yes after rerunning you've got green across the board 🎉

src/volume.ts Outdated Show resolved Hide resolved
@G-Rath G-Rath changed the title fix: allow UInt8Array in pathToFilename feat: support UInt8Array in place of Buffer Jan 9, 2025
@G-Rath G-Rath merged commit 0d3662a into streamich:master Jan 9, 2025
10 checks passed
github-actions bot pushed a commit that referenced this pull request Jan 9, 2025
# [4.16.0](v4.15.4...v4.16.0) (2025-01-09)

### Features

* support `UInt8Array` in place of `Buffer` ([#1083](#1083)) ([0d3662a](0d3662a))
Copy link

github-actions bot commented Jan 9, 2025

🎉 This PR is included in version 4.16.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

readSync and others functions doesn't really supports uint8array
2 participants