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

readFileSync should return a Buffer instead of Uint8Array #256

Closed
borkdude opened this issue Jul 6, 2022 · 7 comments
Closed

readFileSync should return a Buffer instead of Uint8Array #256

borkdude opened this issue Jul 6, 2022 · 7 comments
Labels
bug Something isn't working node.js Compatibility with Node.js APIs

Comments

@borkdude
Copy link

borkdude commented Jul 6, 2022

Repro:

index.mjs

import { readFileSync } from 'fs'

console.log(readFileSync('index.cljs').toString())

index.cljs:

(defn foo []
  (prn :hello))

(foo)

On Node, this prints the contents of index.mjs as text. On bun it prints a bunch of numbers.

See https://github.com/babashka/nbb/blob/main/doc/bun.md for more context on this.

@borkdude
Copy link
Author

borkdude commented Jul 6, 2022

I realize that I maybe should not use toString on the buffer, but use utf-8 as the encoding argument. TIL!

@borkdude
Copy link
Author

borkdude commented Jul 6, 2022

Yep, I was able to work around this using an explicit encoding.

@blackmann
Copy link
Contributor

Please remember to close this issue if you're good

@borkdude
Copy link
Author

borkdude commented Jul 6, 2022 via email

@SheetJSDev
Copy link
Contributor

SheetJSDev commented Jul 9, 2022

The underlying issue is that readFileSync returns a Buffer in NodeJS but currently returns a Uint8Array in Bun:

% cat > bun.mjs <<EOF
import { readFileSync } from 'fs';
console.log(Buffer.isBuffer(readFileSync('bun.mjs')));
EOF

% node bun.mjs     
true
% bun bun.mjs     
false

Explicitly converting to a Buffer "just works" in Node and Bun:

import { readFileSync } from 'fs';
const rFS = path => Buffer.from(readFileSync(path));
console.log(rFS('index.cljs').toString());

Looking at the roadmap https://github.com/Jarred-Sumner/bun/issues/159 , fs hopefully will be changed once Buffer is in a good spot

@Electroid Electroid changed the title readFileSync + toString results in different representation on Node vs bun readFileSync should return a Buffer instead of Uint8Array Nov 1, 2022
@Electroid Electroid added bug Something isn't working node.js Compatibility with Node.js APIs labels Nov 1, 2022
@Electroid
Copy link
Contributor

Yes we should fix this! Thanks @SheetJSDev and @borkdude for reproducing the issue, we're tracking it.

@Jarred-Sumner
Copy link
Collaborator

This has been fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working node.js Compatibility with Node.js APIs
Projects
None yet
Development

No branches or pull requests

5 participants