core icon indicating copy to clipboard operation
core copied to clipboard

TypeError: Invalid URL at buildZipUrl with path instead of absolute url

Open adamdharrington opened this issue 10 months ago • 2 comments

Describe the bug

When using the dts_plugin in an rsbuild basic setup there is one part of the manifest generation that disallows using a path instead of an absolute url for remotes: i.e. '/my-path' instead of 'https://app.com/my-path'.

Since everything 'works' fine with paths instead of absolute urls (except this zip path) I'd hazard this is a bug.

start   Compiling...
Unhandled Rejection Error:  TypeError: Invalid URL
    at new URL (node:internal/url:775:36)
    at buildZipUrl (/Users/xxx/Development/temp/federation_consumer/node_modules/@module-federation/dts-plugin/dist/forkDevWorker.js:1603:21)
    at retrieveRemoteInfo (/Users/xxx/Development/temp/federation_consumer/node_modules/@module-federation/dts-plugin/dist/forkDevWorker.js:1621:24)
    at /Users/xxx/Development/temp/federation_consumer/node_modules/@module-federation/dts-plugin/dist/forkDevWorker.js:1645:26
    at Array.reduce (<anonymous>)
    at resolveRemotes (/Users/xxx/Development/temp/federation_consumer/node_modules/@module-federation/dts-plugin/dist/forkDevWorker.js:1642:24)
    at retrieveHostConfig (/Users/xxx/Development/temp/federation_consumer/node_modules/@module-federation/dts-plugin/dist/forkDevWorker.js:1658:32)
    at getLocalRemoteNames (/Users/xxx/Development/temp/federation_consumer/node_modules/@module-federation/dts-plugin/dist/forkDevWorker.js:2389:36)
    at forkDevWorker (/Users/xxx/Development/temp/federation_consumer/node_modules/@module-federation/dts-plugin/dist/forkDevWorker.js:2472:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
federation_consumer Process(64855) unhandledRejection, mf server will exit...

The error stems from the following code where the remoteUrl is passed to new URL (which requires a second 'baseUrl' param is passed a path).

dts-plugin

const buildZipUrl = (hostOptions: Required<HostOptions>, url: string) => {
  const remoteUrl = new URL(url);


  if (remoteUrl.href.includes(MANIFEST_EXT)) {
    return undefined;
  }
  const pathnameWithoutEntry = remoteUrl.pathname
    .split('/')
    .slice(0, -1)
    .join('/');
  remoteUrl.pathname = `${pathnameWithoutEntry}/${hostOptions.remoteTypesFolder}.zip`;


  return remoteUrl.href;
};

Suggested solution

Since the baseUrl will only be used when the first argument is a path and otherwise ignored I changed the locally installed (compiled) package to construct the url with an arbitrary domain which I check for and remove if present in the final url, something like:

const buildZipUrl = (hostOptions: Required<HostOptions>, url: string) => {
  const remoteUrl = new URL(url, 'https://example.com');
   /*...*/
  return remoteUrl.origin.includes('example.com') ? remoteUrl.pathname : remoteUrl.href;
};

Reproduction

https://github.com/adamdharrington/mod-fed-dts-error/blob/main/rsbuild.config.ts#L24

Used Package Manager

npm

System Info

System:
    OS: macOS 13.2.1
    CPU: (8) x64 Intel(R) Core(TM) i7-1060NG7 CPU @ 1.20GHz
    Memory: 1.92 GB / 16.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 20.9.0 - ~/.volta/tools/image/node/20.9.0/bin/node
    Yarn: 1.22.21 - ~/.volta/bin/yarn
    npm: 10.1.0 - ~/.volta/tools/image/node/20.9.0/bin/npm
  Browsers:
    Brave Browser: 123.1.64.113
    Chrome: 124.0.6367.62
    Safari: 16.3

Validations

adamdharrington avatar Apr 28 '24 18:04 adamdharrington

can you pass file:// protocol?

ScriptedAlchemy avatar Apr 29 '24 04:04 ScriptedAlchemy

Yes, that works. And I'm assuming you would never want the file url href returned from buildZipUrl?

Something like:

const buildZipUrl = (hostOptions: Required<HostOptions>, url: string) => {
  const remoteUrl = new URL(url, 'file://');
   /*...*/
  return !remoteUrl.host ? remoteUrl.pathname : remoteUrl.href;
};

I can't open a PR right now unfortunately, (pnpm install failing on node-gyp, probably something with my python environment but I can't look into it now).

adamdharrington avatar Apr 29 '24 09:04 adamdharrington