Skip to content

module: change default resolver to not throw on unknown scheme#47824

Merged
nodejs-github-bot merged 12 commits into
nodejs:mainfrom
giltayar:default-resolve-no-throw-scheme
May 19, 2023
Merged

module: change default resolver to not throw on unknown scheme#47824
nodejs-github-bot merged 12 commits into
nodejs:mainfrom
giltayar:default-resolve-no-throw-scheme

Conversation

@giltayar
Copy link
Copy Markdown
Contributor

@giltayar giltayar commented May 2, 2023

This is an example toy loader that supports HTTP[S]. This code should work (in a simplistic way):

export async function load(url, context, nextLoad) {
  if (url.startsWith('http://') || url.startsWith('https://')) {
    const response = await fetch(url, {redirect: 'follow'})
    const source = await response.text()

    return {shortCircuit: true, format: 'module',source}
  }
  return await nextLoad(url, context)
}

Unfortunately, this won't work. If you use it, you will get an ERR_UNSUPPORTED_ESM_URL_SCHEME because the default Node.js resolver doesn't support HTTPS, and throws on unknown schemes. So unfortunately I had to write that this loader needs a resolver, even though the only thing it does is support loading HTTPS. And the resolver I had to wrote was non-trivial, and IIUC replicates some of what the default resolver does:

export async function resolve(specifier, context, nextResolve) {
  if (isBareSpecifier(specifier))
    return await nextResolve(specifier, context)

  const url = new URL(specifier, context.parentURL).href

  if (url.startsWith('http://') || url.startsWith('https://'))
    return {url, shortCircuit: true}

  return await nextResolve(specifier, context)
}

Thinking about it more, I realized that the only reason I need to write this resolver is that Node.js doesn't recognize the HTTP scheme in its resolver. But if Node.js would have not recognized the scheme in the loader, then I wouldn't have needed to write this resolver at all.

So my point is this: both checks for unknown scheme and unknown file extension should move from the default Node.js resolver to the default Node.js loader. This will obviate the need for some loaders to implement a resolve which mostly replicates the Node.js logic, just for the sake of avoiding these exceptions.

This PR implements exactly that.

Fixes nodejs/loaders#138

Notable change:

The ESM loader no longer throws on unknown protocol/file extension in the resolve phase, so import.meta.resolve behavior matches the behavior of other runtimes. This change is potentially for loader hooks authors that rely on the old behavior.

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. notable-change PRs with changes that should be highlighted in changelogs.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

default resolve should not throw ERR_UNSUPPORTED_ESM_URL_SCHEME

8 participants