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

Jump to bottom

Use nullish coalescing to respect zero positional reads #40716

Closed
wants to merge 1 commit into from
Closed

Use nullish coalescing to respect zero positional reads #40716

wants to merge 1 commit into from

Conversation

mihilmy
Copy link
Contributor

@mihilmy mihilmy commented Nov 3, 2021 •
edited by aduh95

More details available at #40715

Fixes: #40715
Fixes: #40699

@nodejs-github-bot 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
@Mesteery
Copy link
Member

Mesteery commented Nov 3, 2021

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
Copy link
Contributor Author

mihilmy commented Nov 3, 2021 •
edited

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

@devsnek
Copy link
Member

devsnek commented Nov 3, 2021

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

This was linked to issues Nov 3, 2021
operator || #40699
Closed
[fs/promises] FileHandle.read does not respect reading from position=0 #40715
Closed
mihilmy added a commit to mihilmy/node that referenced this pull request Nov 4, 2021
@mihilmy
fs: nullish coalescing to respect zero positional reads
e1d3e9d 
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
@ronag 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 github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 4, 2021
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@mihilmy
Copy link
Contributor Author

mihilmy commented Nov 4, 2021

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.

@github-actions github-actions bot mentioned this pull request Nov 5, 2021
Open
43 tasks
@tniessen
Copy link
Member

tniessen commented Nov 5, 2021

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

This was referenced Nov 6, 2021
Open
Open
@github-actions github-actions bot mentioned this pull request Nov 8, 2021
Open
46 tasks
@joyeecheung joyeecheung mentioned this pull request Nov 8, 2021
Closed
@github-actions github-actions bot mentioned this pull request Nov 9, 2021
Open
23 tasks
@Trott Trott added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 9, 2021
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 11, 2021
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@github-actions github-actions bot mentioned this pull request Nov 12, 2021
Open
35 tasks
@nodejs-github-bot
Copy link
Collaborator

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

@mihilmy @Trott
fs: nullish coalescing to respect zero positional reads
2037ee8 
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 <ronagy@icloud.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Evan Lucas <evanlucas@me.com>
@Trott
Copy link
Member

Trott commented Nov 12, 2021

Landed in 2037ee8

@Trott
Copy link
Member

Trott commented Nov 12, 2021

Thanks for the contribution! 🎉

@Trott Trott closed this Nov 12, 2021
This was referenced Nov 13, 2021
Open
Open
Open
Open
Open
targos pushed a commit that referenced this pull request Nov 21, 2021
@mihilmy @targos
fs: nullish coalescing to respect zero positional reads
a5a1691 
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 <ronagy@icloud.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Evan Lucas <evanlucas@me.com>
@targos targos mentioned this pull request Nov 26, 2021
Merged
@christian102094 christian102094 mentioned this pull request Jan 17, 2022
Closed
richardlau pushed a commit that referenced this pull request Jan 25, 2022
@mihilmy @richardlau
fs: nullish coalescing to respect zero positional reads
fc328f1 
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 <ronagy@icloud.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Evan Lucas <evanlucas@me.com>
@richardlau richardlau mentioned this pull request Jan 25, 2022
Merged
danielleadams pushed a commit that referenced this pull request Jan 30, 2022
@mihilmy @danielleadams
fs: nullish coalescing to respect zero positional reads
7e221ae 
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 <ronagy@icloud.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Evan Lucas <evanlucas@me.com>
danielleadams pushed a commit that referenced this pull request Feb 1, 2022
@mihilmy @danielleadams
fs: nullish coalescing to respect zero positional reads
0377edc 
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 <ronagy@icloud.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Evan Lucas <evanlucas@me.com>
@danielleadams danielleadams mentioned this pull request Feb 1, 2022
Merged
mwalbeck pushed a commit to mwalbeck/docker-jellyfin-livestream that referenced this pull request Mar 16, 2022
@mwalbeck
Update Node.js to v14.19.0 (#97)
7fa000e 
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 - [#&#8203;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 - [#&#8203;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 - [#&#8203;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 - [#&#8203;40280](nodejs/node#40280)

##### Other Notable Changes

-   \[[`0d448eaab5`](nodejs/node@0d448eaab5)] - **(SEMVER-MINOR)** **crypto**: make FIPS related options always available (Vít Ondruch) [#&#8203;36341](nodejs/node#36341)
-   \[[`004eafbebf`](nodejs/node@004eafbebf)] - **(SEMVER-MINOR)** **lib**: add unsubscribe method to non-active DC channels (simon-id) [#&#8203;40433](nodejs/node#40433)
-   \[[`625be7585d`](nodejs/node@625be7585d)] - **(SEMVER-MINOR)** **lib**: add return value for DC channel.unsubscribe (simon-id) [#&#8203;40433](nodejs/node#40433)
-   \[[`607bc74eae`](nodejs/node@607bc74eae)] - **(SEMVER-MINOR)** **module**: support pattern trailers (Guy Bedford) [#&#8203;39635](nodejs/node#39635)
-   \[[`f74fe2a59c`](nodejs/node@f74fe2a59c)] - **(SEMVER-MINOR)** **src**: make napi_create_reference accept symbol (JckXia) [#&#8203;39926](nodejs/node#39926)

##### Commits

-   \[[`0231ffa501`](nodejs/node@0231ffa501)] - **build**: add `--without-corepack` (Jonah Snider) [#&#8203;41060](nodejs/node#41060)
-   \[[`5389b8ab05`](nodejs/node@5389b8ab05)] - **crypto**: update root certificates (Richard Lau) [#&#8203;40280](nodejs/node#40280)
-   \[[`0d448eaab5`](nodejs/node@0d448eaab5)] - **(SEMVER-MINOR)** **crypto**: make FIPS related options always available (Vít Ondruch) [#&#8203;36341](nodejs/node#36341)
-   \[[`cd20ecc7cb`](nodejs/node@cd20ecc7cb)] - **deps**: upgrade Corepack to 0.10 (Maël Nison) [#&#8203;40374](nodejs/node#40374)
-   \[[`737df75e17`](nodejs/node@737df75e17)] - **(SEMVER-MINOR)** **deps**: add corepack (Maël Nison) [#&#8203;39608](nodejs/node#39608)
-   \[[`b85aa5a143`](nodejs/node@b85aa5a143)] - **deps**: upgrade npm to 6.14.16 (Ruy Adorno) [#&#8203;41603](nodejs/node#41603)
-   \[[`2755d391a5`](nodejs/node@2755d391a5)] - **deps**: update ICU to 70.1 (Michaël Zasso) [#&#8203;40658](nodejs/node#40658)
-   \[[`3089326d89`](nodejs/node@3089326d89)] - **deps**: update archs files for OpenSSL-1.1.1m (Richard Lau) [#&#8203;41173](nodejs/node#41173)
-   \[[`59da7c12aa`](nodejs/node@59da7c12aa)] - **deps**: upgrade openssl sources to 1.1.1m (Richard Lau) [#&#8203;41173](nodejs/node#41173)
-   \[[`cede1f26f6`](nodejs/node@cede1f26f6)] - **deps**: add -fno-strict-aliasing flag to libuv (Daniel Bevenius) [#&#8203;40631](nodejs/node#40631)
-   \[[`4477da858f`](nodejs/node@4477da858f)] - **doc**: fix corepack grammar for `--force` flag (Steven) [#&#8203;40762](nodejs/node#40762)
-   \[[`5971d58600`](nodejs/node@5971d58600)] - **doc**: add missing YAML tag in `esm.md` (Antoine du Hamel) [#&#8203;41516](nodejs/node#41516)
-   \[[`e903798ae1`](nodejs/node@e903798ae1)] - **doc**: add note regarding unfinished TLA (Antoine du Hamel) [#&#8203;41434](nodejs/node#41434)
-   \[[`a90defebcf`](nodejs/node@a90defebcf)] - **esm**: make `process.exit()` default to exit code 0 (Gang Chen) [#&#8203;41388](nodejs/node#41388)
-   \[[`fc328f1ab0`](nodejs/node@fc328f1ab0)] - **fs**: nullish coalescing to respect zero positional reads (Omar El-Mihilmy) [#&#8203;40716](nodejs/node#40716)
-   \[[`004eafbebf`](nodejs/node@004eafbebf)] - **(SEMVER-MINOR)** **lib**: add unsubscribe method to non-active DC channels (simon-id) [#&#8203;40433](nodejs/node#40433)
-   \[[`625be7585d`](nodejs/node@625be7585d)] - **(SEMVER-MINOR)** **lib**: add return value for DC channel.unsubscribe (simon-id) [#&#8203;40433](nodejs/node#40433)
-   \[[`2c365961d0`](nodejs/node@2c365961d0)] - **module**: support pattern trailers for imports field (Guy Bedford) [#&#8203;40041](nodejs/node#40041)
-   \[[`607bc74eae`](nodejs/node@607bc74eae)] - **(SEMVER-MINOR)** **module**: support pattern trailers (Guy Bedford) [#&#8203;39635](nodejs/node#39635)
-   \[[`f74fe2a59c`](nodejs/node@f74fe2a59c)] - **(SEMVER-MINOR)** **src**: make napi_create_reference accept symbol (JckXia) [#&#8203;39926](nodejs/node#39926)
-   \[[`b050c65885`](nodejs/node@b050c65885)] - **src**: add option to disable loading native addons (Dominic Elm) [#&#8203;39977](nodejs/node#39977)
-   \[[`c1695ac68a`](nodejs/node@c1695ac68a)] - **tools**: update certdata.txt (Richard Lau) [#&#8203;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.

---

 - [ ] <!-- rebase-check -->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 <bot@walbeck.it>
Co-committed-by: renovate-bot <bot@walbeck.it>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lpinca lpinca lpinca approved these changes

@evanlucas evanlucas evanlucas approved these changes

@ronag ronag ronag approved these changes

@tniessen tniessen tniessen approved these changes

@BridgeAR BridgeAR BridgeAR approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

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