Set ERR_PACKAGE_PATH_NOT_EXPORTED code on errors when bailing

[paul-soporan]

Node throws an ERR_PACKAGE_PATH_NOT_EXPORTED error when "Package exports do not define or permit a target subpath in the package for the given module".

For improved compatibility with the Node ecosystem, I think it would be a good idea for resolve.exports to set error.code to ERR_PACKAGE_PATH_NOT_EXPORTED when bailing, so that packages can continue checking the error code to tell "path not exported" errors apart from other errors.

Ref:

• https://nodejs.org/api/esm.html#esm_resolver_algorithm
• https://github.com/nodejs/node/blob/2af43f65055de2817eb7907d3a3d3b3a3de127f5/lib/internal/modules/esm/resolve.js#L321

[lukeed]

Thanks, I wasn't sure where I stood on this.
The important part, at least for now, is throwing at all.

Introducing identical error codes might be a slippery slope that then leads to identical error messages and error stack/details – both of which I either don't want to do, or can't. Node errors are also tied to the file system, giving the filepath(s) checked as well as the parentURL.

I could see adding my own error codes that differentiate the two Error types thrown here. That way it'd be easier to differentiate the Error w/o matching the error message text.

In any event, I was hoping to reach some kind of consensus from bundlers and/or Node teams to figure out what should be done here.

[paul-soporan]

Thanks for sharing your perspective on this, I can give you my perspective on how PnP, a Node installation strategy (where I'm currently trying to implement exports support via resolve.exports - it's been working very great so far) does the error code part.

PnP patches the Node resolution and throws more detailed errors on undeclared dependencies (and other things). Those errors have a custom message, stack, details, an error.pnpCode property that differentiates our own errors, and also the regular error.code property which is set to what Node would throw (in our case MODULE_NOT_FOUND), because we need it to maintain compatibility with various tools that use the error.code property to tell errors apart.

Because of this, in PnP we'll need to assign the Node error code even if resolve.exports won't. I opened this discussion here because I felt like it would make sense for an exports resolver to throw an error that at least has the code corresponding to the Node implementation. 🤔

[lukeed]

Yup, understood! I'll try to get some more eyes on this and see if there's some kind of group decision to be made here.

[developit]

I think this would be nice. Given that this package is mostly used in cli/tooling/server environments, it feels like it's worth the extra bytes. It provides for a potential future scenario where resolve() throws an error for some other reason (malformed map key, invalid path value, etc).

For a library, it'd be preferable to avoid subclassing Error though. Just something like Object.assign(Error('xyz'), { code: 'ERR_PACKAGE_PATH_NOT_EXPORTED' }) feels like enough.

[lukeed]

Thanks! Right, I would just add a code key instead of throwing custom error types.

@paul-soporan already raised #5 for throwing on invalid target(s), but I don't think resolve() is the right time/place for that. I don't see this as an "exports" validator utility.