fix(build): make relative imports in amd/cjs relative to the current module

[dschmidt]

Description

make relative imports in amd/cjs relative to the current module instead of the baseURI.

I've added an override to customRelativeUrlMechanisms to avoid modifying the copied code from rollup.

Fixes #20860

As far as I can tell rollup itself uses module.uri to replace import.meta.url in amd modules. So I assume it's okay to use it.
https://github.com/rollup/rollup/blob/ce6cb93098850a46fa242e37b74a919e99a5de28/src/ast/nodes/MetaProperty.ts#L209

An alternative could be to use resolve.toUrl(module.id) but it seems unneccessary if other parts of the code already rely on module.uri.

FWIW I've verified that import.meta.url gets transpiled to new URL(module.uri, document.baseURI).href in my setup with amd modules. Using import.meta.url directly for the imports here does not work as it happens too late in the transpilation process (getting errors that module may not be used outside of module scope).

I'm a bit lost regarding a regression test as I couldn't find a test for the relative path behavior that I could simply adjust... if you require me to implement one, could you give me a few pointers how to implement this specific one?

[sapphi-red]

Would you provide concrete examples of require.toUrl(relativePath), module.uri, and document.baseURI?
Rollup has been using the same code for years, so I guess there's a reason.
Have you tried setting the baseUrl option?

[dschmidt]

Sure, no problem. I've used this code:

const customRelativeUrlMechanisms = {
  ...relativeUrlMechanisms,
  // override amd to use module.uri instead of document.baseURI
  amd: (relativePath) => {
    if (relativePath[0] !== '.') relativePath = './' + relativePath
    return getResolveUrl(
      `require.toUrl('${escapeId(relativePath)}'), (() => {
        console.log('relativePath', '${relativePath}')
        console.log('require.toUrl(relativePath)', require.toUrl('${relativePath}'))
        console.log('module.id', module.id)
        console.log('module.uri', module.uri)
        console.log('document.baseURI', document.baseURI)

        return new URL(module.uri, document.baseURI).href
      })()`,
    )
  },
  'worker-iife': (relativePath) =>
    getResolveUrl(
      `'${escapeId(partialEncodeURIPath(relativePath))}', self.location.href`,
    ),
} as const satisfies Record<string, (relativePath: string) => string>

to produce this output:

relativePath ../assets/zip-web-worker-CqnRs-V2.js
unzip.js:4681 require.toUrl(relativePath) ./../assets/zip-web-worker-CqnRs-V2.js
unzip.js:4682 module.id https://host.docker.internal:9200/assets/apps/unzip/js/unzip.js
unzip.js:4683 module.uri https://host.docker.internal:9200/assets/apps/unzip/js/unzip.js
unzip.js:4684 document.baseURI https://host.docker.internal:9200/

[dschmidt]

I'm not sure about the baseUrl - is it possible to have multiple instances of requirejs?
We have multiple apps with individual baseUrls so to speak.
If I do

requirejs.config({
    baseUrl: '/assets/apps/unzip/'
  });

that's only valid for a single app but not for all others.

No idea why rollup implemented it this way. Maybe module.uri wasn't always available.
Usually it probably just doesn't matter. I have to admit my use case is a bit of an edge case - but it's one that is supported by system and es modules.
I think it's a valid way to define "relative paths" to be relative to some base uri, but then all module systems should implement it that way.

[dschmidt]

If you insist, I can also send a PR to Rollup and we can check their feedback 😅

[sapphi-red]

Thank you for the examples and the PR to Rollup. I think it makes sense. Just in case, I'd like to wait for a week for the feedback on Rollup side.

[dschmidt]

Makes sense. I understand that while being a small change in terms of LoC, the change could potentlally be delicate and I certainly don't want to break others.
If we can fix it in rollup and vite and keep the code in sync, that would of course be the best outcome.

If rollup people see a problem, I'm happy to discuss alternatives with you and them.
Until then we can certainly wait. As I wrote above it's an issue we have a workaround for, so there's absolutely no urgency to fix it... but it would be so much nicer if we could just use ?url without manual tinkering with the url (and ?worker and import other assets... )