Skip to content

Commit

Permalink
fix(commonjs)!: Correctly infer module name for any separator (#924)
Browse files Browse the repository at this point in the history
* test(commonjs): Add test for different path separators

Add test for module IDs that use a path separator that is different from
`path.sep`.

Previously, when a module whose file name is `index.js` was resolved to
a module ID containing different path separators, the commonjs plugin
generated a variable name containing the module's absolute path.

* fix(commonjs)!: Correctly infer module name for any separator

BREAKING CHANGES:
Correctly infer the module name from the module ID, regardless of the
path separator used in the module ID and the value of `path.sep`.

This generates variable names like `react` instead of
`C__testViteApp_node_modules_react` when a module ID of
`C:/test-vite-app/node_modules/react/index.js` is given on Windows.
  • Loading branch information
pastelmind committed Jul 30, 2021
1 parent 31aad90 commit f408497
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 3 deletions.
5 changes: 2 additions & 3 deletions packages/commonjs/src/utils.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint-disable import/prefer-default-export */

import { basename, dirname, extname, sep } from 'path';
import { basename, dirname, extname } from 'path';

import { makeLegalIdentifier } from '@rollup/pluginutils';

Expand All @@ -27,8 +27,7 @@ export function getName(id) {
if (name !== 'index') {
return name;
}
const segments = dirname(id).split(sep);
return makeLegalIdentifier(segments[segments.length - 1]);
return makeLegalIdentifier(basename(dirname(id)));
}

export function normalizePathSlashes(path) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports.a = 1;
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
const foo = require("./foo");
console.log(foo.a);
29 changes: 29 additions & 0 deletions packages/commonjs/test/test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* eslint-disable line-comment-position, no-new-func, no-undefined */
import * as path from 'path';
import os from 'os';

import resolve from '@rollup/plugin-node-resolve';

Expand Down Expand Up @@ -728,3 +729,31 @@ test('does not affect subsequently created instances when called with `requireRe

t.is(code1, code2);
});

// This test works only on Windows, which treats both forward and backward
// slashes as path separators
if (os.platform() === 'win32') {
test('supports both forward and backward slash as path separator in directory-based modules', async (t) => {
const bundle = await rollup({
input: 'fixtures/samples/module-path-separator/main.js',
plugins: [
// Ad-hoc plugin that reverses the path separator of foo/index.js
{
name: 'test-path-separator-reverser',
async resolveId(source, importer) {
if (source.endsWith('foo')) {
const fullPath = path.resolve(path.dirname(importer), source, 'index.js');
// Ensure that the module ID uses a non-default path separator
return fullPath.replace(/[\\/]/g, (sep) => (sep === '/' ? '\\' : '/'));
}
return null;
}
},
commonjs()
]
});

const code = await getCodeFromBundle(bundle);
t.regex(code, /\bfoo = {}/);
});
}

0 comments on commit f408497

Please sign in to comment.