-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
errors: fix ERR_MISSING_DYNAMIC_INSTANTIATE_HOOK #21493
Conversation
bd6bb89
to
1a1b056
Compare
bbcb61d
to
1fcf5e1
Compare
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.
sorry about that 😃
on a side note, how come our eslint rules didn't pick up throwing a non-internal-error
Ping @ChALkeR there is a small conflict. |
@targos, sorry, I am not feeling well and have only my phone right now. 🌡️ |
@ChALkeR sure. I hope you get better soon! I'm happy to take it over. Can I push to your branch? |
@targos, thanks! |
1fcf5e1
to
d7fa6c2
Compare
This restores a broken and erroneously removed error, which was accidentially renamed to ERR_MISSING_DYNAMIC_INTSTANTIATE_HOOK (notice the "INTST" vs "INST") in 921fb84 (PR nodejs#16874) and then had documentation and implementation removed under the old name in 6e1c25c (PR nodejs#18857), as it appeared unused. This error code never worked or was documented under the mistyped name ERR_MISSING_DYNAMIC_INTSTANTIATE_HOOK, so renaming it back to ERR_MISSING_DYNAMIC_INSTANTIATE_HOOK is a semver-patch fix. Refs: nodejs#21440 Refs: nodejs#21470 Refs: nodejs#16874 Refs: nodejs#18857
d7fa6c2
to
75bf6a8
Compare
@targos Thanks for that again! |
No problem! |
Landed in 7bdc694 |
This restores a broken and erroneously removed error, which was accidentially renamed to ERR_MISSING_DYNAMIC_INTSTANTIATE_HOOK (notice the "INTST" vs "INST") in 921fb84 (PR #16874) and then had documentation and implementation removed under the old name in 6e1c25c (PR #18857), as it appeared unused. This error code never worked or was documented under the mistyped name ERR_MISSING_DYNAMIC_INTSTANTIATE_HOOK, so renaming it back to ERR_MISSING_DYNAMIC_INSTANTIATE_HOOK is a semver-patch fix. Refs: #21440 Refs: #21470 Refs: #16874 Refs: #18857 PR-URL: #21493 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ron Korving <ron@ronkorving.nl> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Node.js v10.x is affected. |
This restores a broken and erroneously removed error, which was accidentially renamed to ERR_MISSING_DYNAMIC_INTSTANTIATE_HOOK (notice the "INTST" vs "INST") in 921fb84 (PR #16874) and then had documentation and implementation removed under the old name in 6e1c25c (PR #18857), as it appeared unused. This error code never worked or was documented under the mistyped name ERR_MISSING_DYNAMIC_INTSTANTIATE_HOOK, so renaming it back to ERR_MISSING_DYNAMIC_INSTANTIATE_HOOK is a semver-patch fix. Refs: #21440 Refs: #21470 Refs: #16874 Refs: #18857 PR-URL: #21493 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ron Korving <ron@ronkorving.nl> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
This restores a broken and erroneously removed error, which was accidentially renamed to
ERR_MISSING_DYNAMIC_INTSTANTIATE_HOOK
(notice theINTST
vsINST
) in 921fb84 (PR #16874) and then had documentation and implementation removed under the old name in 6e1c25c (PR #18857), as it appeared unused.This error code never worked or was documented under the mistyped name
ERR_MISSING_DYNAMIC_INTSTANTIATE_HOOK
, so renaming it back toERR_MISSING_DYNAMIC_INSTANTIATE_HOOK
is a semver-patch fix.Refs: #21440, #21470, #16874, #18857.
This is a part of the fixes hinted by #21470, which includes some tests for error codes usage and documentation and enforces a stricter format.
This is mostly a revert, and specific tests for esm loader actually emitting that error are not included here.
Generic tests for error codes are included in a separate PR: #21470.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes