-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
esm: restore next<HookName>
's context
as optional arg
#43553
esm: restore next<HookName>
's context
as optional arg
#43553
Conversation
Review requested:
|
@@ -0,0 +1,3 @@ | |||
export async function load(specifier, context, next) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we also have a test for when the return value is null
instead of Promise<null>
?
export async function load(specifier, context, next) { | |
export function load(specifier, context, next) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep! Will do tomorrow (alternatively in the tidy follow-up I'm about to do)
PS gaah, yah killin me with the drip-feeding 😜
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM assuming #43558 fixes the failing test
4227327
to
1f380d1
Compare
1f380d1
to
cb738d6
Compare
Landed in 1f6d005 |
PR-URL: #43553 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #43553 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #43553 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs/node#43553 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This PR ports some internal code improvements otherwise lost when #43363 was reverted. It also contains a small bugfix wherein the
context
argument ofnext<HookName>()
(ordefault<HookName>()
prior to chaining) was actually optional.