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.

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

Jump to bottom

fix(ext/node): dispatch beforeExit/exit events irrespective of listeners #23382

Merged
merged 5 commits into from
Apr 16, 2024
Merged

fix(ext/node): dispatch beforeExit/exit events irrespective of listeners #23382

merged 5 commits into from
Apr 16, 2024

Conversation

satyarohith
Copy link
Member

@satyarohith satyarohith commented Apr 15, 2024 •
edited
Loading

Closes #23342
Closes #21757

bartlomieju
bartlomieju reviewed Apr 15, 2024
View reviewed changes
Copy link
Member

@bartlomieju bartlomieju left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Nice work, this one is close. Looks like you uncovered a bug in deno coverage that writes data for modules using node: specifiers. We will have to fix it before landing this PR - I think you need to add a check in cli/tools/coverage/mod.rs on line 73 to discard modules starting with node:

ext/node/polyfills/process.ts Show resolved Hide resolved
ext/node/polyfills/process.ts Show resolved Hide resolved
runtime/worker.rs Outdated Show resolved Hide resolved
runtime/worker.rs Outdated Show resolved Hide resolved
runtime/worker.rs Outdated Show resolved Hide resolved
runtime/worker.rs Show resolved Hide resolved
tests/integration/node_compat_tests.rs Show resolved Hide resolved
bartlomieju
bartlomieju reviewed Apr 16, 2024
View reviewed changes
cli/worker.rs Outdated Show resolved Hide resolved
cli/worker.rs Outdated Show resolved Hide resolved
tests/testdata/node/events_order.ts Outdated Show resolved Hide resolved
@satyarohith satyarohith enabled auto-merge (squash) April 16, 2024 13:21
@satyarohith satyarohith merged commit 50223c5 into main Apr 16, 2024
17 checks passed
@satyarohith satyarohith deleted the satya/fix_23342 branch April 16, 2024 13:45
littledivy pushed a commit to littledivy/deno that referenced this pull request Apr 19, 2024
@satyarohith @littledivy
fix(ext/node): dispatch beforeExit/exit events irrespective of listen…
b484f49 
…ers (denoland#23382)

Closes denoland#23342
Closes denoland#21757
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bartlomieju bartlomieju bartlomieju approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ext/node: "exit" event emitted only if event listeners are registered npm:signal-exit package not working
2 participants
@satyarohith @bartlomieju