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

Null-proofing in files_watcher #47694

Closed
wants to merge 21 commits into from
Closed

Null-proofing in files_watcher #47694

wants to merge 21 commits into from

Conversation

sosoba
Copy link
Contributor

@sosoba sosoba commented Apr 24, 2023

Regardles to documentation:

filename is not always guaranteed to be provided. Therefore, don't assume that filename argument is always provided in the callback, and have some fallback logic if it is null.

Fix for #47692

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Apr 24, 2023
Wath can pass null as filename
doc/api/fs.md Outdated Show resolved Hide resolved
@MoLow MoLow added the watch-mode Issues and PRs related to watch mode label Apr 24, 2023
@MoLow
Copy link
Member

MoLow commented Apr 24, 2023

@MoLow
Copy link
Member

MoLow commented Apr 24, 2023

additionally if you have a repro can you add it as a test?

@sosoba
Copy link
Contributor Author

sosoba commented Apr 24, 2023

additionally if you have a repro can you add it as a test?

Unfortunately not. The program that generated the event:

{eventType: 'change', fileName: null}

while running node --watch at the same time is not open code..

@sosoba
Copy link
Contributor Author

sosoba commented Apr 24, 2023

@sosoba you will also need to remove the merge commits, see https://github.com/nodejs/node/blob/main/doc/contributing/pull-requests.md#step-5-rebase

Rebased.

@MoLow
Copy link
Member

MoLow commented Apr 24, 2023

Unfortunately not. The program that generated the event:

then please add a test case to https://github.com/nodejs/node/blob/main/test/parallel/test-watch-mode-files_watcher.mjs

@sosoba sosoba closed this Feb 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ci PRs that need a full CI run. watch-mode Issues and PRs related to watch mode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants