[nodefs] Remove compatibility code for ancient node versions #23316
Conversation
| @@ -123,15 +119,6 @@ addToLibrary({ | |||
| var stat; | |||
| NODEFS.tryFSOperation(() => stat = fs.lstatSync(path)); | |||
| if (NODEFS.isWindows) { | |||
| // node.js v0.10.20 doesn't report blksize and blocks on Windows. Fake | |||
There was a problem hiding this comment.
We do still support MIN_NODE_VERSION=101900 (10.19.0) so if we want to drop this I guess we would need to bump that minimum too?
There was a problem hiding this comment.
Can you add some stat tests to test-node-compat in .circleci/config.yml?
There was a problem hiding this comment.
How's 6f92152 look?
|
Can you mention nodefs in the title somehow? Maybe prefix with |
| if (flags["fs"]) { | ||
| flags = flags["fs"]; | ||
| } | ||
| var flags = process.binding("constants")["fs"]; |
There was a problem hiding this comment.
I was going to suggest single quotes here but it looks like this file users double-quote a lot, so maybe we leave as is.
.circleci/config.yml
Outdated
| @@ -780,6 +780,8 @@ jobs: | |||
| other.test_full_js_library* | |||
| core2.test_hello_world | |||
| core2.test_fs_write_rawfs" | |||
| core2.*nodefs* | |||
| core2.*rawfs* | |||
There was a problem hiding this comment.
Thats quite a few tests but maybe thats fine. Can you remove core2.test_fs_write_rawfs above?
And you need to move the closing quote the last line.
|
It looks like not all of those tests are actually runnable on the oldest version of node that we support (v10.19.0). Can you perhaps revert the We can work on adding all of them in followup PRs perhaps? |
|
For the closure compiler errors I think you can maybe add |
|
I also wonder how long we want to keep supporting EOL node versions. Node v10 has been EOL for about 4 years. But we'd probably need to bump to version 16 to get a big help. Node 14 has only been EOL since April 2023 which might be a bit recent to drop. |
hoodmane
changed the title
That is certainly valid discussion and we should consider bumping the min version. Perhaps you could open an issue regarding that? |
This is an automatic change generated by tools/maint/rebaseline_tests.py. The following (13) test expectation files were updated by running the tests with `--rebaseline`: ``` other/codesize/test_codesize_cxx_ctors1.jssize: 20271 => 20282 [+11 bytes / +0.05%] other/codesize/test_codesize_cxx_ctors2.gzsize: 8325 => 8326 [+1 bytes / +0.01%] other/codesize/test_codesize_cxx_ctors2.jssize: 20239 => 20250 [+11 bytes / +0.05%] other/codesize/test_codesize_cxx_except.jssize: 24039 => 24050 [+11 bytes / +0.05%] other/codesize/test_codesize_cxx_except_wasm.gzsize: 8288 => 8291 [+3 bytes / +0.04%] other/codesize/test_codesize_cxx_except_wasm.jssize: 20164 => 20175 [+11 bytes / +0.05%] other/codesize/test_codesize_cxx_except_wasm_exnref.gzsize: 8288 => 8291 [+3 bytes / +0.04%] other/codesize/test_codesize_cxx_except_wasm_exnref.jssize: 20164 => 20175 [+11 bytes / +0.05%] other/codesize/test_codesize_cxx_lto.gzsize: 8357 => 8356 [-1 bytes / -0.01%] other/codesize/test_codesize_cxx_lto.jssize: 20347 => 20357 [+10 bytes / +0.05%] other/codesize/test_codesize_cxx_mangle.jssize: 24039 => 24050 [+11 bytes / +0.05%] other/codesize/test_codesize_cxx_noexcept.jssize: 20271 => 20282 [+11 bytes / +0.05%] other/codesize/test_codesize_files_js_fs.jssize: 18817 => 18828 [+11 bytes / +0.06%] Average change: +0.04% (-0.01% - +0.06%) ```
hoodmane
force-pushed
the
windows-nodefs-cleanup
branch
from
b234dee to
e93545c
Compare
No description provided.