Skip to content

Ensure compatibility with older versions of Node.js #21863

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
May 1, 2024
Merged

Ensure compatibility with older versions of Node.js #21863

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8f6e33a
Ensure compatibility with older versions of Node.js
kleisauke Apr 30, 2024
65a6ff3
Prefer old API instead
kleisauke Apr 30, 2024
99b0518
Add test
kleisauke Apr 30, 2024
1da12e4
Avoid magic numbers
kleisauke Apr 30, 2024
8dafceb
Fix tests
kleisauke Apr 30, 2024
e3a284b
Merge branch 'main' into old-nodejs-compat
kleisauke May 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions src/library_nodefs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,14 +276,22 @@ addToLibrary({
// Node.js < 6 compatibility: node errors on 0 length reads
if (length === 0) return 0;
try {
return fs.readSync(stream.nfd, new Int8Array(buffer.buffer, offset, length), { position: position });
#if MIN_NODE_VERSION < 131300
return fs.readSync(stream.nfd, new Int8Array(buffer.buffer, offset, length), 0, length, position);
Copy link
Collaborator

@sbc100 sbc100 Apr 30, 2024

Choose a reason for hiding this comment

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

Do newer versions of node also support this old pattern? If so, can we not just use this older pattern everwhere? Is there any downside? It looks like it creates less gabage since you don't need to create a new option object each call.

If newer versions don't support this pattern then I suppose we would want a runtime check rather than a compile time one.

Copy link
Collaborator Author

@kleisauke kleisauke Apr 30, 2024

Choose a reason for hiding this comment

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

Newer versions of Node.js ought to support this too. I guarded this for code size reasons, but perhaps that is negligible.

Copy link
Collaborator Author

@kleisauke kleisauke Apr 30, 2024

Choose a reason for hiding this comment

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

Commit 65a6ff3 prefers the old API instead.

#else
return fs.readSync(stream.nfd, new Int8Array(buffer.buffer, offset, length), { position });
#endif
} catch (e) {
throw new FS.ErrnoError(NODEFS.convertNodeCode(e));
}
},
write(stream, buffer, offset, length, position) {
try {
return fs.writeSync(stream.nfd, new Int8Array(buffer.buffer, offset, length), { position: position });
#if MIN_NODE_VERSION < 180300
return fs.writeSync(stream.nfd, new Int8Array(buffer.buffer, offset, length), 0, length, position);
#else
return fs.writeSync(stream.nfd, new Int8Array(buffer.buffer, offset, length), { position });
#endif
} catch (e) {
throw new FS.ErrnoError(NODEFS.convertNodeCode(e));
}
Expand Down
12 changes: 10 additions & 2 deletions src/library_noderawfs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,11 @@ addToLibrary({
}
var seeking = typeof position != 'undefined';
if (!seeking && stream.seekable) position = stream.position;
var bytesRead = fs.readSync(stream.nfd, new Int8Array(buffer.buffer, offset, length), { position: position });
#if MIN_NODE_VERSION < 131300
var bytesRead = fs.readSync(stream.nfd, new Int8Array(buffer.buffer, offset, length), 0, length, position);
#else
var bytesRead = fs.readSync(stream.nfd, new Int8Array(buffer.buffer, offset, length), { position });
#endif
// update position marker when non-seeking
if (!seeking) stream.position += bytesRead;
return bytesRead;
Expand All @@ -165,7 +169,11 @@ addToLibrary({
}
var seeking = typeof position != 'undefined';
if (!seeking && stream.seekable) position = stream.position;
var bytesWritten = fs.writeSync(stream.nfd, new Int8Array(buffer.buffer, offset, length), { position: position });
#if MIN_NODE_VERSION < 180300
var bytesWritten = fs.writeSync(stream.nfd, new Int8Array(buffer.buffer, offset, length), 0, length, position);
#else
var bytesWritten = fs.writeSync(stream.nfd, new Int8Array(buffer.buffer, offset, length), { position });
#endif
// update position marker when non-seeking
if (!seeking) stream.position += bytesWritten;
return bytesWritten;
Expand Down
8 changes: 8 additions & 0 deletions src/library_wasmfs_node.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,11 @@ addToLibrary({
try {
// TODO: Cache open file descriptors to guarantee that opened files will
// still exist when we try to access them.
#if MIN_NODE_VERSION < 131300
let nread = fs.readSync(fd, new Int8Array(HEAPU8.buffer, buf_p, len), 0, len, pos);
#else
let nread = fs.readSync(fd, new Int8Array(HEAPU8.buffer, buf_p, len), { position: pos });
#endif
{{{ makeSetValue('nread_p', 0, 'nread', 'i32') }}};
} catch (e) {
if (!e.code) throw e;
Expand All @@ -202,7 +206,11 @@ addToLibrary({
try {
// TODO: Cache open file descriptors to guarantee that opened files will
// still exist when we try to access them.
#if MIN_NODE_VERSION < 180300
let nwritten = fs.writeSync(fd, new Int8Array(HEAPU8.buffer, buf_p, len), 0, len, pos);
#else
let nwritten = fs.writeSync(fd, new Int8Array(HEAPU8.buffer, buf_p, len), { position: pos });
#endif
{{{ makeSetValue('nwritten_p', 0, 'nwritten', 'i32') }}};
} catch (e) {
if (!e.code) throw e;
Expand Down
21 changes: 21 additions & 0 deletions third_party/closure-compiler/node-externs/fs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,16 @@ fs.fsyncSync = function(fd) {};
*/
fs.write = function(fd, buffer, offset, length, position, callback) {};

/**
* @param {*} fd
* @param {*} buffer
* @param {number=} offset
* @param {number=} length
* @param {number=} position
* @return {number}
*/
fs.writeSync = function(fd, buffer, offset, length, position) {};

/**
* @param {*} fd
* @param {*} buffer
Expand All @@ -388,6 +398,17 @@ fs.writeSync = function(fd, buffer, options) {};
*/
fs.read = function(fd, buffer, offset, length, position, callback) {};

/**
* @param {*} fd
* @param {*} buffer
* @param {number} offset
* @param {number} length
* @param {number} position
* @return {number}
* @nosideeffects
*/
fs.readSync = function(fd, buffer, offset, length, position) {};

/**
* @param {*} fd
* @param {*} buffer
Expand Down