Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.3k
Revert "esm: convert resolve hook to synchronous"#43526
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
Revert "esm: convert resolve hook to synchronous" #43526
Uh oh!
There was an error while loading. Please reload this page.
Conversation
This reverts commit 90b634a.
nodejs-github-bot commented Jun 21, 2022
Review requested:
|
GeoffreyBooth commented Jun 21, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Request fast track. @nodejs/loaders |
Fast-track has been requested by @GeoffreyBooth. Please 👍 to approve. |
guybedford left a comment
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.
As discussed in the loaders meeting, this takes the pressure off the design space for now, and hopefully with some progress on threaded loaders, syncification of the resolver can become possible that way. And this PR can always be added back if it comes to that.
aduh95 left a comment
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.
RSLGTM
This comment was marked as outdated.
This comment was marked as outdated.
GeoffreyBooth commented Jun 21, 2022
cc @juanarbol, please make sure that this revert is processed before the next release (in other words, let’s make sure that #43363 gets reverted and therefore doesn’t go out in the next release). |
richardlau commented Jun 22, 2022
If #43363 hasn't gone out in a release we can mark it with |
nodejs-github-bot commented Jun 22, 2022
nodejs-github-bot commented Jun 22, 2022
nodejs-github-bot commented Jun 22, 2022
nodejs-github-bot commented Jun 22, 2022
Landed in 3c04034 |
GeoffreyBooth commented Jun 22, 2022
@richardlau This is the revert; we want it to land in any branch where the original PR landed . . . ? |
richardlau commented Jun 22, 2022
@GeoffreyBooth AFAIK #43363 hasn't landed on any of the staging branches. |
This reverts commit 90b634a. PR-URL: nodejs#43526 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reverts #43363
Per nodejs/loaders#90
@nodejs/releasers @danielleadams We decided to revert the PR which was previously marked as blocking several other PRs; those other PRs remain unblocked. Apologies for the confusion.