Skip to content

module: allow omitting context in synchronous next hooks - #57056

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
joyeecheung:hooks-merged-context
Feb 18, 2025
Merged

module: allow omitting context in synchronous next hooks#57056
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
joyeecheung:hooks-merged-context

Conversation

@joyeecheung

@joyeecheungjoyeecheung commented Feb 14, 2025

Copy link
Copy Markdown
Member

This aligns the behavior of synchronous hooks with asynchronous hooks by allowing omission of the context parameter in the invocation of next hooks. The contexts are merged along the chain.

Fixes: #57030

@nodejs-github-bot

nodejs-github-bot commented Feb 14, 2025

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/loaders

@nodejs-github-botnodejs-github-bot added module Issues and PRs related to the module subsystem. needs-ci PRs that need a full CI run. labels Feb 14, 2025
@joyeecheung
module: allow omitting context in synchronous next hooks
952c0f7
This aligns the behavior of synchronous hooks with asynchronous
hooks by allowing omission of the context parameter in the
invocation of next hooks. The contexts are merged along the
chain.
@codecov

codecovBot commented Feb 15, 2025•
edited
Loading

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.11%. Comparing base (79f96b6) to head (952c0f7).
Report is 32 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #57056 +/- ##
==========================================
+ Coverage 89.10% 89.11% +0.01% 
==========================================
Files 665 665 Lines 193203 193216 +13 Branches 37220 37222 +2 ==========================================
+ Hits 172158 172190 +32 + Misses 13771 13758 -13 + Partials 7274 7268 -6
Files with missing linesCoverage Δ
lib/internal/modules/customization_hooks.js100.00% <100.00%> (ø)

... and 29 files with indirect coverage changes

JakobJingleheimer
JakobJingleheimer approved these changes Feb 15, 2025
View reviewed changes
Comment threadlib/internal/modules/customization_hooks.js
@joyeecheungjoyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 15, 2025
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 15, 2025
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
mcollina
mcollina approved these changes Feb 16, 2025
View reviewed changes

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

nodejs-github-bot commented Feb 17, 2025•
edited by jasnell
Loading

Copy link
Copy Markdown
Collaborator

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

@joyeecheungjoyeecheung added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 18, 2025
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 18, 2025
@nodejs-github-bot
nodejs-github-bot merged commit ea2004a into nodejs:mainFeb 18, 2025
@nodejs-github-bot

nodejs-github-bot commented Feb 18, 2025

Copy link
Copy Markdown
Collaborator

Landed in ea2004a

@joyeecheungjoyeecheung mentioned this pull request Feb 18, 2025
Closed
acidiney pushed a commit to acidiney/node that referenced this pull request Feb 23, 2025
@joyeecheung@acidiney
module: allow omitting context in synchronous next hooks
9f5e862
This aligns the behavior of synchronous hooks with asynchronous
hooks by allowing omission of the context parameter in the
invocation of next hooks. The contexts are merged along the
chain.
PR-URL: nodejs#57056Fixes: nodejs#57030
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Feb 24, 2025
@joyeecheung@targos
module: allow omitting context in synchronous next hooks
d5a994c
This aligns the behavior of synchronous hooks with asynchronous
hooks by allowing omission of the context parameter in the
invocation of next hooks. The contexts are merged along the
chain.
PR-URL: #57056Fixes: #57030
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@github-actionsgithub-actionsBot mentioned this pull request Feb 25, 2025
Merged
targos pushed a commit that referenced this pull request Feb 25, 2025
@joyeecheung@targos
module: allow omitting context in synchronous next hooks
cc1cafd
This aligns the behavior of synchronous hooks with asynchronous
hooks by allowing omission of the context parameter in the
invocation of next hooks. The contexts are merged along the
chain.
PR-URL: #57056Fixes: #57030
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 pushed a commit that referenced this pull request Apr 2, 2025
@joyeecheung@aduh95
module: allow omitting context in synchronous next hooks
861d56c
This aligns the behavior of synchronous hooks with asynchronous
hooks by allowing omission of the context parameter in the
invocation of next hooks. The contexts are merged along the
chain.
PR-URL: #57056Fixes: #57030
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 pushed a commit that referenced this pull request Apr 3, 2025
@joyeecheung@aduh95
module: allow omitting context in synchronous next hooks
4f36fb9
This aligns the behavior of synchronous hooks with asynchronous
hooks by allowing omission of the context parameter in the
invocation of next hooks. The contexts are merged along the
chain.
PR-URL: #57056Fixes: #57030
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@aduh95aduh95 mentioned this pull request Apr 11, 2025
Merged
RafaelGSS pushed a commit that referenced this pull request Apr 16, 2025
@joyeecheung@RafaelGSS
module: allow omitting context in synchronous next hooks
5ad8b35
This aligns the behavior of synchronous hooks with asynchronous
hooks by allowing omission of the context parameter in the
invocation of next hooks. The contexts are merged along the
chain.
PR-URL: #57056Fixes: #57030
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Apr 17, 2025
@joyeecheung@RafaelGSS
module: allow omitting context in synchronous next hooks
41fa7d3
This aligns the behavior of synchronous hooks with asynchronous
hooks by allowing omission of the context parameter in the
invocation of next hooks. The contexts are merged along the
chain.
PR-URL: #57056Fixes: #57030
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollinamcollinamcollina approved these changes
@jasnelljasnelljasnell approved these changes
@JakobJingleheimerJakobJingleheimerJakobJingleheimer approved these changes
+1 more reviewer
@jsumners-nrjsumners-nrjsumners-nr approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

moduleIssues and PRs related to the module subsystem.needs-ciPRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

TypeError: Cannot add property source, object is not extensible

6 participants

@joyeecheung@nodejs-github-bot@mcollina@jasnell@JakobJingleheimer@jsumners-nr