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

feat(pnp): support consolidated ESM loader hooks #3603

Merged
merged 3 commits into from Oct 21, 2021

Conversation

merceyz
Copy link
Member

@merceyz merceyz commented Oct 20, 2021

What's the problem this PR addresses?

The ESM loader added in #2161 doesn't work in node@>=16.12.0 (released a few hours after we merged) due to nodejs/node#37468

How did you fix it?

Updated the loader to support the new load hook

Checklist

  • I have read the Contributing Guide.
  • I have set the packages that need to be released for my changes to be effective.
  • I will check that all automated PR checks pass before the PR gets reviewed.

Comment on lines +7 to +11
export async function getSource(
urlString: string,
context: { format: string },
defaultGetSource: typeof getSource,
): Promise<{ source: string }> {
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we should contribute those types to DefinitelyTyped?

Copy link
Member Author

Choose a reason for hiding this comment

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

Some of these hooks are removed already so we would have to define some of them either way

@merceyz merceyz merged commit d66d54a into master Oct 21, 2021
@merceyz merceyz deleted the merceyz/feat/consolidated-esm-loader branch October 21, 2021 16:47
@merceyz merceyz added the esm label Oct 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants