FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Revert "esm: convert `resolve` hook to synchronous" by JakobJingleheimer · Pull Request #43526 · nodejs/node · GitHub

/ node Public

Revert "esm: convert resolve hook to synchronous" - #43526

Merged
nodejs-github-bot merged 1 commit into
mainfrom
revert-43363-esm/synchronous-resolve-hook
Jun 22, 2022
ConversationCommits (1)ChecksFiles changed
Merged

Revert "esm: convert resolve hook to synchronous"#43526
nodejs-github-bot merged 1 commit into
mainfrom
revert-43363-esm/synchronous-resolve-hook

Conversation

JakobJingleheimer commented Jun 21, 2022

Copy link
Copy Markdown
Member

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.

JakobJingleheimer added esm Issues and PRs related to the ECMAScript Modules implementation. loaders Issues and PRs related to ES module loaders labels Jun 21, 2022

nodejs-github-bot commented Jun 21, 2022

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/modules

nodejs-github-bot added the needs-ci PRs that need a full CI run. label Jun 21, 2022

GeoffreyBooth commented Jun 21, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Request fast track. @nodejs/loaders

GeoffreyBooth added the fast-track PRs that do not need to wait for 48 hours to land. label Jun 21, 2022

github-actions Bot commented Jun 21, 2022

Copy link
Copy Markdown
Contributor

Fast-track has been requested by @GeoffreyBooth. Please 👍 to approve.

guybedford approved these changes Jun 21, 2022
View reviewed changes

guybedford left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 approved these changes Jun 21, 2022
View reviewed changes

aduh95 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

RSLGTM

This comment was marked as outdated.

Sign in to view

GeoffreyBooth commented Jun 21, 2022

Copy link
Copy Markdown
Member

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).

nodejs/Release#737

richardlau commented Jun 22, 2022

Copy link
Copy Markdown
Member

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).

nodejs/Release#737

If #43363 hasn't gone out in a release we can mark it with dont-land-on-* labels.

github-actions Bot mentioned this pull request Jun 22, 2022
Open
35 tasks

nodejs-github-bot commented Jun 22, 2022

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Jun 22, 2022

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Jun 22, 2022

Copy link
Copy Markdown
Collaborator

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

JakobJingleheimer added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 22, 2022
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 22, 2022
nodejs-github-bot merged commit 3c04034 into main Jun 22, 2022

nodejs-github-bot commented Jun 22, 2022

Copy link
Copy Markdown
Collaborator

Landed in 3c04034

aduh95 deleted the revert-43363-esm/synchronous-resolve-hook branch June 22, 2022 07:15

GeoffreyBooth commented Jun 22, 2022

Copy link
Copy Markdown
Member

@richardlau This is the revert; we want it to land in any branch where the original PR landed . . . ?

richardlau commented Jun 22, 2022

Copy link
Copy Markdown
Member

@GeoffreyBooth AFAIK #43363 hasn't landed on any of the staging branches.

This was referenced Jun 23, 2022
Open
Open
This was referenced Jun 25, 2022
Open
Open
Open
GeoffreyBooth mentioned this pull request Jul 4, 2022
Merged
3 tasks
RafaelGSS mentioned this pull request Oct 14, 2022
Merged
This was referenced Jan 25, 2024
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

GeoffreyBooth GeoffreyBooth approved these changes
guybedford guybedford approved these changes
aduh95 aduh95 approved these changes
bmeck Awaiting requested review from bmeck
+1 more reviewer
VoltrexKeyva VoltrexKeyva approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

esm Issues and PRs related to the ECMAScript Modules implementation. fast-track PRs that do not need to wait for 48 hours to land. loaders Issues and PRs related to ES module loaders needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

Back | FazBrowse Home | New Git URL