cloudflare
🚀 Feature Request — Provide the `raw-specifier` in the Module fallback service requests
It would be great if the requests that the module fallback server receive could provide the original/raw/unaltered specifier exactly as it was defined in the referrer code.
To give more context we've been experimenting with using the module fallback service alongside the Vite environment api in order to lazily import cjs modules (which we could otherwise not import since vite does not perform any transformation on such code).
Meaning that, in workerd (specifically in the ModuleEvaluator's runExternalModule method), we receive a path such as the following from Vite:
/Users/me/project/node_modules/.pnpm/[email protected]/node_modules/react/cjs/react-jsx-dev-runtime.development.js
that we need to import and run.
The above works fine initially, but can easily break when the module in question includes require/import calls, such for example as:
var React = require('react');
and
var util = require('./utils.js');
The problem being that the above require calls produce respectively the following specifiers and referrers:
specifier: /Users/me/project/node_modules/.pnpm/[email protected]/node_modules/react/cjs/react
referrer: /Users/me/project/node_modules/.pnpm/[email protected]/node_modules/react/cjs/react-jsx-dev-runtime.development.js
and
specifier: /Users/me/project/node_modules/.pnpm/[email protected]/node_modules/react/cjs/utils
referrer: /Users/me/project/node_modules/.pnpm/[email protected]/node_modules/react/cjs/react-jsx-dev-runtime.development.js
and from these we are unable to distinguish/differentiate the fact that react is referring to a dependency package and utils a local file (which for obvious reasons is fundamental for us).
So it would be necessary for us to also have the actual import specifier being used (i.e. "react" and "./utils.js" respectively)
(this would not introduce any breaking change if such specifier would be added to the request's search params or headers potentially as a raw-specifier/original-specifier or something along those lines).
[!Note] We could check the file extension to discern which is which but that is quite brittle as libraries can also end with
.jsetc...
As an example of the specifiers we have in mind, those should follow this comment: https://github.com/cloudflare/workerd/pull/1423#issuecomment-1828786854
(but again probably with a different search param name instead of specifier for backward compatibility)
@jasnell I think the way the specifier is provided to the fallback service in it's semi-resolved way is a bug in workerd, a bug that vitest integration works around by post-processing the specifier: https://github.com/cloudflare/workers-sdk/blob/89b6d7f3832b350b470a981eb3b4388517612363/packages/vitest-pool-workers/src/pool/module-fallback.ts#L207-L231
I don't think that workerd can at all resolve the specifier provided by the user code's import in the cases where we hit the fallback service. In these cases the specifier needs to be resolved by an external service which needs to know what the original specifier, the referrer, and module system type used for the import. These three pieces of info can then be used by the fallback service to do module resolution and if a source for the given module is found return the source code.
Is it intentional that workerd currently does any post processing on the specifier and whether this is intentional or just accidental?
I do think that we want to change this but we'll need to do it in a backwards compatible way so that we don't break the current vitest integration and that's why dario is proposing that the fallback service returns the information using a new key, but other designs are also possible as long as we get the unprocessed specifier and we don't break vitest.
This issue is currently blocking the vite work, which can't proceed until this issue is resolved. Can you please give us an estimate for the fix so that we can plan our next steps? Thanks a bunch!
As discussed in chat, best estimate I can give right now is that I'm hoping to have time to look at the issue later this week.
