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

ESM Hooks: nextResolve & nextLoad can return promises which is not reflected in documentation #49282

Closed
koshic opened this issue Aug 22, 2023 · 4 comments · Fixed by #49265
Closed
Labels
doc Issues and PRs related to the documentations. esm Issues and PRs related to the ECMAScript Modules implementation.

Comments

@koshic
Copy link

koshic commented Aug 22, 2023

Affected URL(s)

https://nodejs.org/dist/latest-v20.x/docs/api/esm.html#hooks

Description of the problem

image

But in the reality nextResolve (and nextLoad) can return Promises, which is highly confusing: as an example, try...catch around 'return nextResolve(...)' will never catch the error if nextResolve returns rejected Promise. The same thing with access to nextResolve(...) result properties.

In both cases await keyword is mandatory to get predictable results.

@koshic koshic added the doc Issues and PRs related to the documentations. label Aug 22, 2023
@VoltrexKeyva VoltrexKeyva added the esm Issues and PRs related to the ECMAScript Modules implementation. label Aug 22, 2023
@aduh95
Copy link
Contributor

aduh95 commented Aug 22, 2023

Would you like to send a PR?

@koshic
Copy link
Author

koshic commented Aug 22, 2023

Would you like to send a PR?

Sorry, I don't have enough time 😒

@GeoffreyBooth
Copy link
Member

Our linter complains if we do return await, because it’s unnecessary, so you might see code like return nextResolve(...). But I think our examples always await the next* call when we’re using the returned value.

@koshic What changes in the docs would make things more obvious to you? Would the changes discussed in #49265 (comment) have been sufficient?

@koshic
Copy link
Author

koshic commented Aug 22, 2023

Our linter complains if we do return await, because it’s unnecessary...

Small off topic: do you mean https://eslint.org/docs/latest/rules/no-return-await? 'This rule was deprecated in ESLint v8.46.0 with no replacement' (c)

@koshic What changes in the docs would make things more obvious to you? Would the changes discussed in #49265 (comment) have been sufficient?

Looks better than existing docs due to 'async'. My issue is a bit late, thx! )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. esm Issues and PRs related to the ECMAScript Modules implementation.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants