fix: stop running into null pathname errors

[codepunkt]

Replace url.parse (legacy) with new URL

Description

Starting from node 16.15.0 (see nodejs/node#42196), url.parse now trims leading and trailing C0 control characters. leading C0 control characters are recommended to be prefixed for resolved virtual module ids (see https://vitejs.dev/guide/api-plugin.html#virtual-modules-convention).

This means that if we use an URL like \0virtual:mypackage-somedata from a plugins resolveId method, pathname will be null and you will run into an error like this starting from node 16.15.0:

This was also noted by @bjuriewicz in discussions: #8967

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

[netlify]

✅ Deploy Preview for vite-docs-main canceled.

Name	Link
🔨 Latest commit	a56744b
🔍 Latest deploy log	https://app.netlify.com/sites/vite-docs-main/deploys/62cc871c2f19090008e5713d

[codepunkt]

Excuse me for taking 3 commits to do this. I was a little dismissive and thought I could do it in GitHub UI only. Turns out doing it in a proper UI and running tests alongside would've been better :)

[codepunkt]

@patak-dev @sapphi-red can you please have a look?

[codepunkt]

I’m not sure what’s happening, but work is continuing on other PRs while this is a serious issue for anyone implementing things on top of vite and using newer node versions.

Is anything unclear?

[bjuriewicz]

I think it would be great if the fix would prevent existing code from crashing when changing minor Node.js version.

[sapphi-red]

IIUC ModuleGraph::resolveUrl should not be called with \0foo. All \0 prefixed ids should be replaced to /@id/__x00__{id}. So the problem here is that \0 prefixed id is directly passed to ModuleGraph::resolveUrl.

Where is this ModuleGraph::resolveUrl called from?

[codepunkt]

I think it would be great if the fix would prevent existing code from crashing when changing minor Node.js version.

It does.

[codepunkt]

IIUC ModuleGraph::resolveUrl should not be called with \0foo. All \0 prefixed ids should be replaced to /@id/__x00__{id}. So the problem here is that \0 prefixed id is directly passed to ModuleGraph::resolveUrl.

Where is this ModuleGraph::resolveUrl called from?

Thanks for taking a look!

I've updated my project to 3.0.0 and the problem still persists. ModuleGraph::resolveUrl is called from ModuleGraph::getModuleById, which is called from doTransform in transformRequest. The transformRequest seems to be invoked from both the transformMiddleware aswell as the import analysis plugin.

ModuleGraph::resolveUrl is called from other places multiple times with the correctly replaced /@id/__x00__{id} aswell, but the getModuleById call is just called with \x00{id}.

I pushed a repro repo (not very simplified unfortunately) here: https://github.com/wilsonjs/wilson2

Steps to reproduce:

• pnpm i
• pnpm run build
• cd docs && pnpm run dev

What's important to remember: This works up until node 16.15.0. Starting with that version, it breaks because URL::parse was changed.

Maybe the problem here is that ModuleGraph::resolveUrl should always be called with /@id/__x00__{id}. But even when that's the problem and we can fix it, URL::parse is still legacy and can be replaced with new URL. So why not incorporate this fix, but also debug the origin of the \x00{id} resolveUrl calls?

[sapphi-red]

Thanks for the detailed explanation. I was trying to understand the issue more accurately.

[sapphi-red]

I think this changes the behavior of Line 243.

$ node -e "console.log(require('url').parse('\0virtual:aa'))" # node 16.14.1
Url {
  protocol: null,
  slashes: null,
  auth: null,
  host: null,
  port: null,
  hostname: null,
  hash: null,
  search: null,
  query: null,
  pathname: '\x00virtual:aa',
  path: '\x00virtual:aa',
  href: '\x00virtual:aa'
}
$ node -e "console.log(require('url').parse('\0virtual:aa'))" # node 16.16.0
Url {
  protocol: 'virtual:',
  slashes: null,
  auth: null,
  host: 'aa',
  port: null,
  hostname: 'aa',
  hash: null,
  search: null,
  query: null,
  pathname: null,
  path: null,
  href: 'virtual:aa'
}
$ node -e "console.log(new URL('\0virtual:aa', 'relative://'))" # node 16.16.0
URL {
  href: 'virtual:aa',
  origin: 'null',
  protocol: 'virtual:',
  username: '',
  password: '',
  host: '',
  hostname: '',
  port: '',
  pathname: 'aa',
  search: '',
  searchParams: URLSearchParams {},
  hash: ''
}

'\x00virtual:aa' // url.parse with node 16.14.1
'aa' // new URL with node 16.16.0

[codepunkt]

Yes, it does. But it has a pathname now, while url.parse has null.

[codepunkt]

Do we need the pathname and thus the returned url to be the same as before? As far as I've seen it's used as a key to urlToModuleMap, which should work inside of ModuleGraph.

However, it's also used in import-analysis plugin and urlToModuleMap is read from in ssrStacktrace, which might be problematic. Tests don't seem to cover that, though - they're all green.

[sapphi-red]

Do we need the pathname and thus the returned url to be the same as before?

I'm not familiar to code around here enough to say no. But I suppose it will break things when multiple urls got the same key for urlToModuleMap.

[codepunkt]

Do you known who could help? I’m surprised this is not a big issue to projects building with vite yet 🙈

[patak-dev]

I'll review it in the next days. We'll release 3.0.1 with current fixes to main tomorrow, and then review this issue to resolve it in 3.0.2. Thanks for digging into this @codepunkt

[codepunkt]

You're welcome. Thanks to you, @sapphi-red and all other regular maintainers!
If there's anything I can do to help, let me know!