Use nullish coalescing to respect zero positional reads #40716

mihilmy · 2021-11-03T19:24:30Z

More details available at #40715

Mesteery · 2021-11-03T19:30:37Z

Welcome and thanks you for the pull request!

Could you add a test? For the commit message: https://github.com/nodejs/node/blob/master/doc/guides/contributing/pull-requests.md#commit-message-guidelines.

mihilmy · 2021-11-03T19:43:58Z

@Mesteery Could you point me to where the tests for the fs/promises module is?

devsnek · 2021-11-03T19:55:33Z

the files are in test/parallel most of them are named like test-fs-promises-foobar.js

When the file read position is moved passing zero is not respected and `null` is used instead. PR fixes the issues by using nullish coalescing which will return the rhs only when the lhs is `null` or `undefined`; respecting the zero. Fixes: nodejs#40715 Refs: nodejs#40716

test/parallel/test-fs-promises-file-handle-read.js

nodejs-github-bot · 2021-11-04T20:08:35Z

CI: https://ci.nodejs.org/job/node-test-pull-request/40713/

nodejs-github-bot · 2021-11-04T21:29:38Z

CI: https://ci.nodejs.org/job/node-test-pull-request/40718/

mihilmy · 2021-11-04T23:32:34Z

Is there anything I can do to fix these failures?

Upon checking the console output I see failures on specific linux flavors, I am not sure if my changes have any OS specific behavior.

tniessen · 2021-11-05T11:49:45Z

These are flaky tests and the failures appear to be unrelated to your changes :)

nodejs-github-bot · 2021-11-11T13:42:09Z

CI: https://ci.nodejs.org/job/node-test-pull-request/40828/

nodejs-github-bot · 2021-11-11T18:35:04Z

CI: https://ci.nodejs.org/job/node-test-pull-request/40832/

nodejs-github-bot · 2021-11-11T19:50:33Z

CI: https://ci.nodejs.org/job/node-test-pull-request/40835/

nodejs-github-bot · 2021-11-12T00:44:51Z

CI: https://ci.nodejs.org/job/node-test-pull-request/40841/

When the file read position is moved passing zero is not respected and `null` is used instead. PR fixes the issues by using nullish coalescing which will return the rhs only when the lhs is `null` or `undefined`; respecting the zero. Fixes: nodejs#40715 PR-URL: nodejs#40716 Fixes: nodejs#40699 Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Evan Lucas <[email protected]>

Trott · 2021-11-12T18:37:11Z

Landed in 2037ee8

Trott · 2021-11-12T18:37:26Z

Thanks for the contribution! 🎉

When the file read position is moved passing zero is not respected and `null` is used instead. PR fixes the issues by using nullish coalescing which will return the rhs only when the lhs is `null` or `undefined`; respecting the zero. Fixes: #40715 PR-URL: #40716 Fixes: #40699 Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Evan Lucas <[email protected]>

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [node](https://github.com/nodejs/node) | stage | minor | `14.18.3-bullseye-slim` -> `14.19.0-bullseye-slim` | --- ### Release Notes <details> <summary>nodejs/node</summary> ### [`v14.19.0`](https://github.com/nodejs/node/releases/v14.19.0) [Compare Source](nodejs/node@v14.18.3...v14.19.0) ##### Notable Changes ##### Corepack Node.js now includes Corepack, a script that acts as a bridge between Node.js projects and the package managers they are intended to be used with during development. In practical terms, **Corepack will let you use Yarn and pnpm without having to install them** - just like what currently happens with npm, which is shipped in Node.js by default. Please head over to the [Corepack documentation page](https://nodejs.org/dist/latest-v14.x/docs/api/corepack.html) for more information on how to use it. Contributed by Maël Nison - [#39608](nodejs/node#39608) ##### ICU updated ICU has been updated to 70.1. This updates timezone database to 2021a3, including bringing forward the start for DST for Jordan from March to February. Contributed by Michaël Zasso - [#40658](nodejs/node#40658) ##### New option to disable loading of native addons A new command line option `--no-addons` has been added to disallow loading of native addons. Contributed by Dominic Elm - [#39977](nodejs/node#39977) ##### Updated Root Certificates Root certificates have been updated to those from Mozilla's Network Security Services 3.71. Contributed by Richard Lau - [#40280](nodejs/node#40280) ##### Other Notable Changes - \[[`0d448eaab5`](nodejs/node@0d448eaab5)] - **(SEMVER-MINOR)** **crypto**: make FIPS related options always available (Vít Ondruch) [#36341](nodejs/node#36341) - \[[`004eafbebf`](nodejs/node@004eafbebf)] - **(SEMVER-MINOR)** **lib**: add unsubscribe method to non-active DC channels (simon-id) [#40433](nodejs/node#40433) - \[[`625be7585d`](nodejs/node@625be7585d)] - **(SEMVER-MINOR)** **lib**: add return value for DC channel.unsubscribe (simon-id) [#40433](nodejs/node#40433) - \[[`607bc74eae`](nodejs/node@607bc74eae)] - **(SEMVER-MINOR)** **module**: support pattern trailers (Guy Bedford) [#39635](nodejs/node#39635) - \[[`f74fe2a59c`](nodejs/node@f74fe2a59c)] - **(SEMVER-MINOR)** **src**: make napi_create_reference accept symbol (JckXia) [#39926](nodejs/node#39926) ##### Commits - \[[`0231ffa501`](nodejs/node@0231ffa501)] - **build**: add `--without-corepack` (Jonah Snider) [#41060](nodejs/node#41060) - \[[`5389b8ab05`](nodejs/node@5389b8ab05)] - **crypto**: update root certificates (Richard Lau) [#40280](nodejs/node#40280) - \[[`0d448eaab5`](nodejs/node@0d448eaab5)] - **(SEMVER-MINOR)** **crypto**: make FIPS related options always available (Vít Ondruch) [#36341](nodejs/node#36341) - \[[`cd20ecc7cb`](nodejs/node@cd20ecc7cb)] - **deps**: upgrade Corepack to 0.10 (Maël Nison) [#40374](nodejs/node#40374) - \[[`737df75e17`](nodejs/node@737df75e17)] - **(SEMVER-MINOR)** **deps**: add corepack (Maël Nison) [#39608](nodejs/node#39608) - \[[`b85aa5a143`](nodejs/node@b85aa5a143)] - **deps**: upgrade npm to 6.14.16 (Ruy Adorno) [#41603](nodejs/node#41603) - \[[`2755d391a5`](nodejs/node@2755d391a5)] - **deps**: update ICU to 70.1 (Michaël Zasso) [#40658](nodejs/node#40658) - \[[`3089326d89`](nodejs/node@3089326d89)] - **deps**: update archs files for OpenSSL-1.1.1m (Richard Lau) [#41173](nodejs/node#41173) - \[[`59da7c12aa`](nodejs/node@59da7c12aa)] - **deps**: upgrade openssl sources to 1.1.1m (Richard Lau) [#41173](nodejs/node#41173) - \[[`cede1f26f6`](nodejs/node@cede1f26f6)] - **deps**: add -fno-strict-aliasing flag to libuv (Daniel Bevenius) [#40631](nodejs/node#40631) - \[[`4477da858f`](nodejs/node@4477da858f)] - **doc**: fix corepack grammar for `--force` flag (Steven) [#40762](nodejs/node#40762) - \[[`5971d58600`](nodejs/node@5971d58600)] - **doc**: add missing YAML tag in `esm.md` (Antoine du Hamel) [#41516](nodejs/node#41516) - \[[`e903798ae1`](nodejs/node@e903798ae1)] - **doc**: add note regarding unfinished TLA (Antoine du Hamel) [#41434](nodejs/node#41434) - \[[`a90defebcf`](nodejs/node@a90defebcf)] - **esm**: make `process.exit()` default to exit code 0 (Gang Chen) [#41388](nodejs/node#41388) - \[[`fc328f1ab0`](nodejs/node@fc328f1ab0)] - **fs**: nullish coalescing to respect zero positional reads (Omar El-Mihilmy) [#40716](nodejs/node#40716) - \[[`004eafbebf`](nodejs/node@004eafbebf)] - **(SEMVER-MINOR)** **lib**: add unsubscribe method to non-active DC channels (simon-id) [#40433](nodejs/node#40433) - \[[`625be7585d`](nodejs/node@625be7585d)] - **(SEMVER-MINOR)** **lib**: add return value for DC channel.unsubscribe (simon-id) [#40433](nodejs/node#40433) - \[[`2c365961d0`](nodejs/node@2c365961d0)] - **module**: support pattern trailers for imports field (Guy Bedford) [#40041](nodejs/node#40041) - \[[`607bc74eae`](nodejs/node@607bc74eae)] - **(SEMVER-MINOR)** **module**: support pattern trailers (Guy Bedford) [#39635](nodejs/node#39635) - \[[`f74fe2a59c`](nodejs/node@f74fe2a59c)] - **(SEMVER-MINOR)** **src**: make napi_create_reference accept symbol (JckXia) [#39926](nodejs/node#39926) - \[[`b050c65885`](nodejs/node@b050c65885)] - **src**: add option to disable loading native addons (Dominic Elm) [#39977](nodejs/node#39977) - \[[`c1695ac68a`](nodejs/node@c1695ac68a)] - **tools**: update certdata.txt (Richard Lau) [#40280](nodejs/node#40280) </details> --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). Reviewed-on: https://git.walbeck.it/mwalbeck/docker-jellyfin-livestream/pulls/97 Co-authored-by: renovate-bot <[email protected]> Co-committed-by: renovate-bot <[email protected]>

nodejs-github-bot added fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels Nov 3, 2021

This was linked to issues Nov 3, 2021

operator || #40699

Closed

[fs/promises] FileHandle.read does not respect reading from position=0 #40715

Closed

ronag approved these changes Nov 4, 2021

View reviewed changes

mihilmy force-pushed the patch-1 branch from b83faf9 to e1d3e9d Compare November 4, 2021 17:33

lpinca approved these changes Nov 4, 2021

View reviewed changes

lpinca reviewed Nov 4, 2021

View reviewed changes

test/parallel/test-fs-promises-file-handle-read.js Outdated Show resolved Hide resolved

ronag added request-ci Add this label to start a Jenkins CI on a PR. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed needs-ci PRs that need a full CI run. labels Nov 4, 2021

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 4, 2021

BridgeAR approved these changes Nov 4, 2021

View reviewed changes

github-actions bot mentioned this pull request Nov 5, 2021

CI Reliability 2021-11-05 nodejs/reliability#108

Open

43 tasks

tniessen approved these changes Nov 5, 2021

View reviewed changes

This was referenced Nov 6, 2021

CI Reliability 2021-11-06 nodejs/reliability#109

Open

CI Reliability 2021-11-07 nodejs/reliability#110

Open

evanlucas approved these changes Nov 7, 2021

View reviewed changes

github-actions bot mentioned this pull request Nov 8, 2021

CI Reliability 2021-11-08 nodejs/reliability#111

Open

46 tasks

joyeecheung mentioned this pull request Nov 8, 2021

esm: add support for JSON import assertion #40250

Closed

github-actions bot mentioned this pull request Nov 9, 2021

CI Reliability 2021-11-09 nodejs/reliability#112

Open

23 tasks

Trott added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 9, 2021

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 11, 2021

github-actions bot mentioned this pull request Nov 12, 2021

CI Reliability 2021-11-12 nodejs/reliability#115

Open

35 tasks

Trott force-pushed the patch-1 branch from e71cb6c to 2037ee8 Compare November 12, 2021 18:37

Trott closed this Nov 12, 2021

targos mentioned this pull request Nov 26, 2021

v17.2.0 release proposal #40983

Merged

christian102094 mentioned this pull request Jan 17, 2022

Two types of filehandle.read return different values #41558

Closed

richardlau added lts-watch-v14.x labels Jan 17, 2022

richardlau added backported-to-v14.x and removed lts-watch-v14.x labels Jan 25, 2022

richardlau mentioned this pull request Jan 25, 2022

v14.19.0 proposal #41696

Merged

danielleadams mentioned this pull request Feb 1, 2022

v16.14.0 proposal #41804

Merged

richardlau removed the lts-watch-v16.x label Feb 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use nullish coalescing to respect zero positional reads #40716

Use nullish coalescing to respect zero positional reads #40716

mihilmy commented Nov 3, 2021 •

edited by aduh95

Loading

Mesteery commented Nov 3, 2021

mihilmy commented Nov 3, 2021 •

edited

Loading

devsnek commented Nov 3, 2021

nodejs-github-bot commented Nov 4, 2021

nodejs-github-bot commented Nov 4, 2021

mihilmy commented Nov 4, 2021

tniessen commented Nov 5, 2021

nodejs-github-bot commented Nov 11, 2021

nodejs-github-bot commented Nov 11, 2021

nodejs-github-bot commented Nov 11, 2021

nodejs-github-bot commented Nov 12, 2021

Trott commented Nov 12, 2021

Trott commented Nov 12, 2021

Use nullish coalescing to respect zero positional reads #40716

Use nullish coalescing to respect zero positional reads #40716

Conversation

mihilmy commented Nov 3, 2021 • edited by aduh95 Loading

Mesteery commented Nov 3, 2021

mihilmy commented Nov 3, 2021 • edited Loading

devsnek commented Nov 3, 2021

nodejs-github-bot commented Nov 4, 2021

nodejs-github-bot commented Nov 4, 2021

mihilmy commented Nov 4, 2021

tniessen commented Nov 5, 2021

nodejs-github-bot commented Nov 11, 2021

nodejs-github-bot commented Nov 11, 2021

nodejs-github-bot commented Nov 11, 2021

nodejs-github-bot commented Nov 12, 2021

Trott commented Nov 12, 2021

Trott commented Nov 12, 2021

mihilmy commented Nov 3, 2021 •

edited by aduh95

Loading

mihilmy commented Nov 3, 2021 •

edited

Loading