[commonjs] `rootDir` causes commonjs modules to resolve incorrectly

[lucaelin]

I've recently updated a project of mine to use the latest dev-server packages, those that include the commonjs and rollup3 fixes. While doing so, I noticed an issue with commonjs-modules appearing as their absolute file-system paths in the browsers network tab. Upon investigation, I realized that this is somehow related to setting the rootDir option. Removing this options from the config fixes the import.

The broken imports appear to resolve "too far up" in the file system, walking outside of the projects folder, strangely the working non-commonjs imports also seem to resolve "too far up" , but end up working fine.

I've managed to reproduce the issue in this branch: https://github.com/lucaelin/rollup-commonjs-bug/tree/rootDir-commonjs

[justinfagnani]

I have this issue too. Standard imports work, but as soon as I added plugin-commonjs, require() broke with invalid absolute paths.

I tracked it down to some faulty logic in the rollup adapter that's handling the null-byte paths and resolving the paths to be absolute - that logic isn't taking into account the rootDir at all. While I don't think they need to be turned into absolute paths, the fix to them is simple:

The problem is here: https://github.com/modernweb-dev/web/blob/8c0250c6d844767896360d083e743c5a3edc502a/packages/dev-server-rollup/src/rollupAdapter.ts#L272

The fix is to use rootDir:

if (matches) {
    const upDirs = new Array(parseInt(matches[1], 10)).fill('..');
    resolvedImportPath = `\0${path_1.default.resolve(rootDir, ...upDirs, matches[2])}`;
}

I'll put together a PR

[justinfagnani]

For tests, presumably we want something in here: https://github.com/modernweb-dev/web/tree/master/packages/dev-server-rollup/test/node/fixtures/commonjs

To be set up with a root dir. Anyone know how I can make a server configuration just for one test?