add support for custom resolvers

[iambumblehead]

per discussion iambumblehead/resolvewithplus#58, this PR adds support for using different resolvers with esmock. Specifically, a yarn PnP resolver be specified, so that esmock would yarn PnP module resolution rather npm.

const modulePnP = await esmock('../src/modulePnP.js', {
  '../src/moduleIdChildPnP.js' : () => ['a', 'b']
}, null, {
  resolver: pnpapi.resolveRequest
})

[iambumblehead]

When the pipelines pass ... lets consider the changes a bit before merging.

Instead of a list of resolvers, a single resolver definition is probably sufficient, eg,

const modulePnP = await esmock('../src/modulePnP.js', {
  '../src/moduleIdChildPnP.js' : () => ['a', 'b']
}, null, {
-  resolvers: [ pnpapi.resolveRequest ],
+  resolver: pnpapi.resolveRequest
})

also, the configured resolver should probably fully replace resolvewithplus, so that resolvewithplus is never called when a custom resolver is defined.

[iambumblehead]

The added test is a little cryptic and only runs in recent node environments where module.register is defined, but maybe this is sufficient. @koshic what do you think? Please review.

[iambumblehead]

@koshic you have more skill with typescript than I do and I would be glad if you would update the type definitions file

[koshic]

@koshic you have more skill with typescript than I do and I would be glad if you would update the type definitions file

Sure

[koshic]

Type definitions diff:

// or export it at the end of file
export type Resolver = (id: string, parent: string) => string

type Options = {
  strict?: boolean | undefined,
  purge?: boolean | undefined,
  isModuleNotFoundError?: boolean | undefined,
  resolver?: Resolver | undefined
}

For PnP it works like that:

import { pathToFileURL } from "node:url";
import { type Resolver } from "esmock";

const resolver: Resolver = (id, parent) =>
  pathToFileURL(pnpapi.resolveRequest(id, parent)).href; // esmock works with file URLs internally, so...

[koshic]

@iambumblehead or you prefer that I make changes in the branch?

[koshic]

Custom resolver should process the following special module ids:

• builtin Node module id -> should return id as is or 'node:' + id
• 'import' -> should return null (or other falsy) value.

It's ok for first implementation, but in the future we should remove such unnecessary calls - in my understanding, resolver is responsible to resolve only things can be resolved.

[iambumblehead]

agreed and thank you

[koshic]

@iambumblehead fixed resolver return type. I'm moving my project to new esmock version in parallel, and found that mistake.

[iambumblehead]

@koshic thanks for helping me! esmock is updated so that the resolver is not called with builtin moduleIds

[iambumblehead]

changes here might be "ready"

I can't think of anything else to change or add

[koshic]

changes here might be "ready"

Let me to update my local project and re-run all tests with that version, it will take ~15m

[koshic]

@iambumblehead works fine, thx! lets merge it

do you plan to publish new version today?

[iambumblehead]

@koshic the resolver spec does specify handling of builtin module names https://nodejs.org/api/esm.html#resolution-algorithm-specification

PACKAGE_RESOLVE(packageSpecifier, parentURL)
...
3. If packageSpecifier is a Node.js builtin module name, then
. 1. Return the string "node:" concatenated with packageSpecifier.

iow the resolver should be called with builtin moduleIds

[iambumblehead]

@koshic yes let's publish and we could publish again any follow up changes we want

[koshic]

@koshic the resolver spec does specify handling of builtin module names https://nodejs.org/api/esm.html#resolution-algorithm-specification

Good catch ) So, we should answer to the following question: should esmock have some optimizations based on internal knowledge and skip useless queries?

My opinion - why not? I'm trying to imagine situation when we may need to process builtins differently, but without success.

[iambumblehead]

[email protected] is published.

If your resolver is later updated to handle core modules, I would be interested to remove iscoremodule condition from esmock. This way, esmock would be as flexible as possible for any resolver that might be used.

iscoremodule(id) ? addprotocolnode(id) : opt.resolver(id, parent)

psuedo-code using node:module isBuiltin

import { isBuiltin } from 'node:module'

const protocolNodeRe = /^node:/
const addprotocolnode = p => protocolNodeRe.test(p) ? p : `node:${p}`

const resolver = (moduleId, parent) => {
  if (isBuiltin(id)) return addprotocolnode(id)
  // ...
}

[iambumblehead]

Good catch ) So, we should answer to the following question: should esmock have some optimizations based on internal knowledge and skip useless queries?

I don't have a strong opinion actually. maybe we should leave esmock as it is following this PR and not make any changes...

[koshic]

I removed that code (4 lines., hehe) right after you make "do not call custom resolver with builtin moduleId" commit ))

BTW, I'll make PR to the wiki to describe how to use custom resolver with esmock & yarn pnp

[iambumblehead]

I suppose the best reasons to remove the iscoremodule condition would be

• slightly simplifies esmock, esmock calls the resolver with any moduleId
• anticipates that the resolver is spec compliant and handles builtin moduleIds

[iambumblehead]

I removed that code (4 lines., hehe) right after you make "do not call custom resolver with builtin moduleId" commit ))

ok did not see that

[koshic]

• anticipates that the resolver is spec compliant and handles builtin moduleIds

It makes sense... So if we both have concerns about 'too smart esmock', let's remove that magic. For me, It's better to redo some work than keep some non-ideal code.

[iambumblehead]

ok will make a PR shortly