Add support for writing binary to file system#86
Add support for writing binary to file system#86inokawa wants to merge 2 commits intounifiedjs:mainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #86 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 23 23
Lines 3214 3214
=========================================
Hits 3214 3214 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
test/output.js
Outdated
| await t.test('should write binary to a path', async function () { | ||
| const cwd = new URL('simple-structure/', fixtures) | ||
| const stderr = spy() | ||
| const binary = () => new Uint8Array([0xde, 0xad, 0xbe, 0xef]) |
There was a problem hiding this comment.
Wrapper function seems unneeded, can just copy/paste the value twice?
Or, stash the return value directly in the variable?
Also, more vaguely, what’s a good test for binary? What did you pick? Were there alternatives you considered?
There was a problem hiding this comment.
The problem was non-UTF-8 string data was transformed to UTF-8 string data by TextDecoder before saving. I've added a new test with an Uint8Array including UTF-8 string for clarification (it will succeed with/without the fix).
I'm not an expert on handling binary in JS, but I think we can say the data is the same if the values at every indexes in a TypedArray like Uint8Array is the same. I used deepStrictEqual for that. I'm not sure we need to check data wrapped with other TypedArrays like Float16Array also.
Initial checklist
Description of changes
https://github.com/orgs/remarkjs/discussions/1473
This PR removes the stringification of
Uint8Arraybefore writing to the file system, that allows us outputting binary data from compiler (e.g. .docx, images) with cli like remark-cli.