Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly handle upper directories as external dependencies #4419

Merged
merged 7 commits into from Mar 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion build-plugins/esm-dynamic-import.ts
@@ -1,6 +1,6 @@
import type { Plugin } from 'rollup';

export default function addBinShebangAndEsmImport(): Plugin {
export default function esmDynamicImport(): Plugin {
let importFound = false;
return {
generateBundle() {
Expand Down
24 changes: 8 additions & 16 deletions src/Chunk.ts
Expand Up @@ -51,8 +51,8 @@ import {
isDefaultAProperty,
namespaceInteropHelpersByInteropType
} from './utils/interopHelpers';
import { basename, dirname, extname, isAbsolute, normalize, resolve } from './utils/path';
import relativeId, { getAliasName } from './utils/relativeId';
import { dirname, extname, isAbsolute, normalize, resolve } from './utils/path';
import relativeId, { getAliasName, getImportPath } from './utils/relativeId';
import renderChunk from './utils/renderChunk';
import type { RenderOptions } from './utils/renderHelpers';
import { makeUnique, renderNamePattern } from './utils/renderNamePattern';
Expand Down Expand Up @@ -691,11 +691,13 @@ export default class Chunk {
if (dependency instanceof ExternalModule) {
const originalId = dependency.renderPath;
renderedDependency.id = escapeId(
dependency.renormalizeRenderPath ? this.getRelativePath(originalId, false) : originalId
dependency.renormalizeRenderPath
? getImportPath(this.id!, originalId, false, false)
: originalId
);
} else {
renderedDependency.namedExportsMode = dependency.exportMode !== 'default';
renderedDependency.id = escapeId(this.getRelativePath(dependency.id!, false));
renderedDependency.id = escapeId(getImportPath(this.id!, dependency.id!, false, true));
}
}

Expand Down Expand Up @@ -926,12 +928,12 @@ export default class Chunk {
const renderedResolution =
resolution instanceof Module
? `'${escapeId(
this.getRelativePath((facadeChunk || chunk!).id!, stripKnownJsExtensions)
getImportPath(this.id!, (facadeChunk || chunk!).id!, stripKnownJsExtensions, true)
)}'`
: resolution instanceof ExternalModule
? `'${escapeId(
resolution.renormalizeRenderPath
? this.getRelativePath(resolution.renderPath, stripKnownJsExtensions)
? getImportPath(this.id!, resolution.renderPath, stripKnownJsExtensions, false)
: resolution.renderPath
)}'`
: resolution;
Expand Down Expand Up @@ -1212,16 +1214,6 @@ export default class Chunk {
return referencedFiles;
}

private getRelativePath(targetPath: string, stripJsExtension: boolean): string {
let relativePath = normalize(relative(dirname(this.id!), targetPath));
if (stripJsExtension && relativePath.endsWith('.js')) {
relativePath = relativePath.slice(0, -3);
}
if (relativePath === '..') return '../../' + basename(targetPath);
if (relativePath === '') return '../' + basename(targetPath);
return relativePath.startsWith('../') ? relativePath : './' + relativePath;
}

private inlineChunkDependencies(chunk: Chunk): void {
for (const dep of chunk.dependencies) {
if (this.dependencies.has(dep)) continue;
Expand Down
3 changes: 1 addition & 2 deletions src/ExternalModule.ts
Expand Up @@ -87,15 +87,14 @@ export default class ExternalModule {
return [externalVariable];
}

setRenderPath(options: NormalizedOutputOptions, inputBase: string): string {
setRenderPath(options: NormalizedOutputOptions, inputBase: string): void {
this.renderPath =
typeof options.paths === 'function' ? options.paths(this.id) : options.paths[this.id];
if (!this.renderPath) {
this.renderPath = this.renormalizeRenderPath
? normalize(relative(inputBase, this.id))
: this.id;
}
return this.renderPath;
}

suggestName(name: string): void {
Expand Down
2 changes: 1 addition & 1 deletion src/utils/path.ts
@@ -1,5 +1,5 @@
const ABSOLUTE_PATH_REGEX = /^(?:\/|(?:[A-Za-z]:)?[\\|/])/;
const RELATIVE_PATH_REGEX = /^\.?\.\//;
const RELATIVE_PATH_REGEX = /^\.?\.(\/|$)/;

export function isAbsolute(path: string): boolean {
return ABSOLUTE_PATH_REGEX.test(path);
Expand Down
27 changes: 26 additions & 1 deletion src/utils/relativeId.ts
@@ -1,4 +1,5 @@
import { basename, extname, isAbsolute, relative, resolve } from './path';
import { relative } from '../../browser/path';
import { basename, dirname, extname, isAbsolute, normalize, resolve } from './path';

export function getAliasName(id: string): string {
const base = basename(id);
Expand All @@ -16,3 +17,27 @@ export function isPathFragment(name: string): boolean {
name[0] === '/' || (name[0] === '.' && (name[1] === '/' || name[1] === '.')) || isAbsolute(name)
);
}

const UPPER_DIR_REGEX = /^(\.\.\/)*\.\.$/;

export function getImportPath(
importerId: string,
targetPath: string,
stripJsExtension: boolean,
ensureFileName: boolean
): string {
let relativePath = normalize(relative(dirname(importerId), targetPath));
if (stripJsExtension && relativePath.endsWith('.js')) {
relativePath = relativePath.slice(0, -3);
}
if (ensureFileName) {
if (relativePath === '') return '../' + basename(targetPath);
if (UPPER_DIR_REGEX.test(relativePath)) {
return relativePath
.split('/')
.concat(['..', basename(targetPath)])
.join('/');
}
}
return !relativePath ? '.' : relativePath.startsWith('..') ? relativePath : './' + relativePath;
}
2 changes: 1 addition & 1 deletion test/cli/samples/custom-frame/_config.js
Expand Up @@ -9,7 +9,7 @@ module.exports = {
stderr,
'[!] (plugin at position 1) Error: My error.\n' +
'main.js\ncustom code frame\nError: My error.\n' +
' at Object.transform'
' at Object.'
);
assertIncludes(stderr, 'rollup.config.js:11:17');
}
Expand Down
2 changes: 1 addition & 1 deletion test/cli/samples/watch/no-config-file/_config.js
Expand Up @@ -4,7 +4,7 @@ module.exports = {
description: 'watches without a config file',
command: 'rollup main.js --watch --format es --file _actual/main.js',
abortOnStderr(data) {
if (data.includes(`created _actual${path.sep}main.js`)) {
if (data.includes(`created _actual/main.js`)) {
return true;
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/cli/samples/watch/node-config-file/_config.js
Expand Up @@ -4,7 +4,7 @@ module.exports = {
description: 'watches using a node_modules config files',
command: 'rollup --watch --config node:custom',
abortOnStderr(data) {
if (data.includes(`created _actual${path.sep}main.js`)) {
if (data.includes(`created _actual/main.js`)) {
return true;
}
}
Expand Down
3 changes: 2 additions & 1 deletion test/cli/samples/watch/watch-config-early-update/_config.js
Expand Up @@ -45,6 +45,7 @@ module.exports = {
},
after() {
unlinkSync(configFile);
stopUpdate();
},
abortOnStderr(data) {
if (data === 'initial\n') {
Expand All @@ -62,7 +63,7 @@ module.exports = {
);
return false;
}
if (data.includes(`created _actual${path.sep}output2.js`)) {
if (data.includes(`created _actual/output2.js`)) {
stopUpdate();
return true;
}
Expand Down
4 changes: 2 additions & 2 deletions test/cli/samples/watch/watch-config-error/_config.js
Expand Up @@ -26,7 +26,7 @@ module.exports = {
setTimeout(() => unlinkSync(configFile), 300);
},
abortOnStderr(data) {
if (data.includes(`created _actual${path.sep}main1.js`)) {
if (data.includes(`created _actual/main1.js`)) {
setTimeout(
() => atomicWriteFileSync(configFile, 'throw new Error("Config contains errors");'),
600
Expand All @@ -48,7 +48,7 @@ module.exports = {
}, 600);
return false;
}
if (data.includes(`created _actual${path.sep}main2.js`)) {
if (data.includes(`created _actual/main2.js`)) {
return true;
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/cli/samples/watch/watch-config-no-update/_config.js
Expand Up @@ -23,7 +23,7 @@ module.exports = {
unlinkSync(configFile);
},
abortOnStderr(data) {
if (data.includes(`created _actual${path.sep}main.js`)) {
if (data.includes(`created _actual/main.js`)) {
atomicWriteFileSync(configFile, configContent);
// wait some time for the watcher to trigger
return new Promise(resolve => setTimeout(() => resolve(true), 600));
Expand Down
11 changes: 11 additions & 0 deletions test/function/samples/external-directory-import/_config.js
@@ -0,0 +1,11 @@
module.exports = {
description: 'handles using ../ as external import (#4349)',
options: {
external() {
return true;
}
},
context: {
require: id => id
}
};
13 changes: 13 additions & 0 deletions test/function/samples/external-directory-import/main.js
@@ -0,0 +1,13 @@
import foo1 from './';
import foo2 from '../';
import foo3 from '.';
import foo4 from '..';
import foo5 from './index.js';
import foo6 from '../index.js';

assert.strictEqual(foo1, '.');
assert.strictEqual(foo2, '..');
assert.strictEqual(foo3, '.');
assert.strictEqual(foo4, '..');
assert.strictEqual(foo5, './index.js');
assert.strictEqual(foo6, '../index.js');
11 changes: 7 additions & 4 deletions test/misc/misc.js
Expand Up @@ -221,29 +221,32 @@ console.log(x);
input: {
'base/main': 'main.js',
'base/main/feature': 'feature.js',
'base/main/feature/sub': 'subfeature.js'
'base/main/feature/sub': 'subfeature.js',
'base/main/feature/sub/sub': 'subsubfeature.js'
},
plugins: [
loader({
'main.js': 'export function fn () { return "main"; } console.log(fn());',
'feature.js': 'import { fn } from "main.js"; console.log(fn() + " feature");',
'subfeature.js': 'import { fn } from "main.js"; console.log(fn() + " subfeature");'
'subfeature.js': 'import { fn } from "main.js"; console.log(fn() + " subfeature");',
'subsubfeature.js': 'import { fn } from "main.js"; console.log(fn() + " subsubfeature");'
})
]
});
const {
output: [main, feature, subfeature]
output: [main, feature, subfeature, subsubfeature]
} = await bundle.generate({
entryFileNames: `[name]`,
chunkFileNames: `[name]`,
format: 'es'
});
assert.strictEqual(main.fileName, 'base/main');
assert.ok(main.code.startsWith('function fn'));
assert.strictEqual(feature.fileName, 'base/main/feature');
assert.ok(feature.code.startsWith("import { fn } from '../main'"));
assert.strictEqual(subfeature.fileName, 'base/main/feature/sub');
assert.ok(subfeature.code.startsWith("import { fn } from '../../main'"));
assert.strictEqual(subsubfeature.fileName, 'base/main/feature/sub/sub');
assert.ok(subsubfeature.code.startsWith("import { fn } from '../../../main'"));
});

it('throws the proper error on max call stack exception', async () => {
Expand Down