Skip to content

Commit

Permalink
fix(node-resolve)!: mark module as external if resolved module is ext…
Browse files Browse the repository at this point in the history 
…ernal (#799)

BREAKING CHANGES: Now requires rollup@^2.42.0

* fix(node-resolve): mark module as external if resolved module is external

fixes #609

* don't use ?.

* prevent infinite loops

* check both importee and importer in nested this.resolve

and add more tests

* upgrade to rollup 2.42.0 to simplify nested `this.resolve`
  • Loading branch information
@tjenkinson @guybedford
tjenkinson authored and guybedford committed Apr 30, 2021
1 parent 0150c55 commit 3a543df
Show file tree
Hide file tree
Showing 6 changed files with 234 additions and 178 deletions.
4 changes: 2 additions & 2 deletions packages/node-resolve/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
"modules"
],
"peerDependencies": {
"rollup": "^1.20.0||^2.0.0"
"rollup": "^2.42.0"
},
"dependencies": {
"@rollup/pluginutils": "^3.1.0",
Expand All @@ -71,7 +71,7 @@
"@rollup/plugin-commonjs": "^16.0.0",
"@rollup/plugin-json": "^4.1.0",
"es5-ext": "^0.10.53",
"rollup": "^2.23.0",
"rollup": "^2.42.0",
"source-map": "^0.7.3",
"string-capitalize": "^1.0.1"
},
Expand Down
340 changes: 176 additions & 164 deletions packages/node-resolve/src/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,203 +72,215 @@ export function nodeResolve(opts = {}) {
const browserMapCache = new Map();
let preserveSymlinks;

return {
name: 'node-resolve',
const doResolveId = async (context, importee, importer, opts) => {
// strip query params from import
const [importPath, params] = importee.split('?');
const importSuffix = `${params ? `?${params}` : ''}`;
importee = importPath;

const baseDir = !importer || dedupe(importee) ? rootDir : dirname(importer);

// https://github.com/defunctzombie/package-browser-field-spec
const browser = browserMapCache.get(importer);
if (useBrowserOverrides && browser) {
const resolvedImportee = resolve(baseDir, importee);
if (browser[importee] === false || browser[resolvedImportee] === false) {
return { id: ES6_BROWSER_EMPTY };
}
const browserImportee =
browser[importee] ||
browser[resolvedImportee] ||
browser[`${resolvedImportee}.js`] ||
browser[`${resolvedImportee}.json`];
if (browserImportee) {
importee = browserImportee;
}
}

buildStart(options) {
rollupOptions = options;
const parts = importee.split(/[/\\]/);
let id = parts.shift();
let isRelativeImport = false;

if (id[0] === '@' && parts.length > 0) {
// scoped packages
id += `/${parts.shift()}`;
} else if (id[0] === '.') {
// an import relative to the parent dir of the importer
id = resolve(baseDir, importee);
isRelativeImport = true;
}

for (const warning of warnings) {
this.warn(warning);
if (
!isRelativeImport &&
resolveOnly.length &&
!resolveOnly.some((pattern) => pattern.test(id))
) {
if (normalizeInput(rollupOptions.input).includes(importee)) {
return null;
}
return false;
}

({ preserveSymlinks } = options);
},
const importSpecifierList = [];

generateBundle() {
readCachedFile.clear();
isFileCached.clear();
isDirCached.clear();
},
if (importer === undefined && !importee[0].match(/^\.?\.?\//)) {
// For module graph roots (i.e. when importer is undefined), we
// need to handle 'path fragments` like `foo/bar` that are commonly
// found in rollup config files. If importee doesn't look like a
// relative or absolute path, we make it relative and attempt to
// resolve it. If we don't find anything, we try resolving it as we
// got it.
importSpecifierList.push(`./${importee}`);
}

async resolveId(importee, importer, opts) {
if (importee === ES6_BROWSER_EMPTY) {
return importee;
}
// ignore IDs with null character, these belong to other plugins
if (/\0/.test(importee)) return null;
const importeeIsBuiltin = builtins.has(importee);

if (/\0/.test(importer)) {
importer = undefined;
}
if (importeeIsBuiltin) {
// The `resolve` library will not resolve packages with the same
// name as a node built-in module. If we're resolving something
// that's a builtin, and we don't prefer to find built-ins, we
// first try to look up a local module with that name. If we don't
// find anything, we resolve the builtin which just returns back
// the built-in's name.
importSpecifierList.push(`${importee}/`);
}

// strip query params from import
const [importPath, params] = importee.split('?');
const importSuffix = `${params ? `?${params}` : ''}`;
importee = importPath;
// TypeScript files may import '.js' to refer to either '.ts' or '.tsx'
if (importer && importee.endsWith('.js')) {
for (const ext of ['.ts', '.tsx']) {
if (importer.endsWith(ext) && extensions.includes(ext)) {
importSpecifierList.push(importee.replace(/.js$/, ext));
}
}
}

const baseDir = !importer || dedupe(importee) ? rootDir : dirname(importer);
importSpecifierList.push(importee);

const warn = (...args) => context.warn(...args);
const isRequire =
opts && opts.custom && opts.custom['node-resolve'] && opts.custom['node-resolve'].isRequire;
const exportConditions = isRequire ? conditionsCjs : conditionsEsm;

const resolvedWithoutBuiltins = await resolveImportSpecifiers({
importer,
importSpecifierList,
exportConditions,
warn,
packageInfoCache,
extensions,
mainFields,
preserveSymlinks,
useBrowserOverrides,
baseDir,
moduleDirectories,
rootDir,
ignoreSideEffectsForRoot
});

const resolved =
importeeIsBuiltin && preferBuiltins
? {
packageInfo: undefined,
hasModuleSideEffects: () => null,
hasPackageEntry: true,
packageBrowserField: false
}
: resolvedWithoutBuiltins;
if (!resolved) {
return null;
}

// https://github.com/defunctzombie/package-browser-field-spec
const browser = browserMapCache.get(importer);
if (useBrowserOverrides && browser) {
const resolvedImportee = resolve(baseDir, importee);
if (browser[importee] === false || browser[resolvedImportee] === false) {
return ES6_BROWSER_EMPTY;
}
const browserImportee =
browser[importee] ||
browser[resolvedImportee] ||
browser[`${resolvedImportee}.js`] ||
browser[`${resolvedImportee}.json`];
if (browserImportee) {
importee = browserImportee;
const { packageInfo, hasModuleSideEffects, hasPackageEntry, packageBrowserField } = resolved;
let { location } = resolved;
if (packageBrowserField) {
if (Object.prototype.hasOwnProperty.call(packageBrowserField, location)) {
if (!packageBrowserField[location]) {
browserMapCache.set(location, packageBrowserField);
return { id: ES6_BROWSER_EMPTY };
}
location = packageBrowserField[location];
}
browserMapCache.set(location, packageBrowserField);
}

const parts = importee.split(/[/\\]/);
let id = parts.shift();
let isRelativeImport = false;

if (id[0] === '@' && parts.length > 0) {
// scoped packages
id += `/${parts.shift()}`;
} else if (id[0] === '.') {
// an import relative to the parent dir of the importer
id = resolve(baseDir, importee);
isRelativeImport = true;
if (hasPackageEntry && !preserveSymlinks) {
const fileExists = await exists(location);
if (fileExists) {
location = await realpath(location);
}
}

if (
!isRelativeImport &&
resolveOnly.length &&
!resolveOnly.some((pattern) => pattern.test(id))
) {
if (normalizeInput(rollupOptions.input).includes(importee)) {
return null;
idToPackageInfo.set(location, packageInfo);

if (hasPackageEntry) {
if (importeeIsBuiltin && preferBuiltins) {
if (!isPreferBuiltinsSet && resolvedWithoutBuiltins && resolved !== importee) {
context.warn(
`preferring built-in module '${importee}' over local alternative at '${resolvedWithoutBuiltins.location}', pass 'preferBuiltins: false' to disable this behavior or 'preferBuiltins: true' to disable this warning`
);
}
return false;
} else if (jail && location.indexOf(normalize(jail.trim(sep))) !== 0) {
return null;
}
}

const importSpecifierList = [];

if (importer === undefined && !importee[0].match(/^\.?\.?\//)) {
// For module graph roots (i.e. when importer is undefined), we
// need to handle 'path fragments` like `foo/bar` that are commonly
// found in rollup config files. If importee doesn't look like a
// relative or absolute path, we make it relative and attempt to
// resolve it. If we don't find anything, we try resolving it as we
// got it.
importSpecifierList.push(`./${importee}`);
if (options.modulesOnly && (await exists(location))) {
const code = await readFile(location, 'utf-8');
if (isModule(code)) {
return {
id: `${location}${importSuffix}`,
moduleSideEffects: hasModuleSideEffects(location)
};
}
return null;
}
const result = {
id: `${location}${importSuffix}`,
moduleSideEffects: hasModuleSideEffects(location)
};
return result;
};

const importeeIsBuiltin = builtins.has(importee);
return {
name: 'node-resolve',

if (importeeIsBuiltin) {
// The `resolve` library will not resolve packages with the same
// name as a node built-in module. If we're resolving something
// that's a builtin, and we don't prefer to find built-ins, we
// first try to look up a local module with that name. If we don't
// find anything, we resolve the builtin which just returns back
// the built-in's name.
importSpecifierList.push(`${importee}/`);
}
buildStart(options) {
rollupOptions = options;

// TypeScript files may import '.js' to refer to either '.ts' or '.tsx'
if (importer && importee.endsWith('.js')) {
for (const ext of ['.ts', '.tsx']) {
if (importer.endsWith(ext) && extensions.includes(ext)) {
importSpecifierList.push(importee.replace(/.js$/, ext));
}
}
for (const warning of warnings) {
this.warn(warning);
}

importSpecifierList.push(importee);

const warn = (...args) => this.warn(...args);
const isRequire =
opts && opts.custom && opts.custom['node-resolve'] && opts.custom['node-resolve'].isRequire;
const exportConditions = isRequire ? conditionsCjs : conditionsEsm;

const resolvedWithoutBuiltins = await resolveImportSpecifiers({
importer,
importSpecifierList,
exportConditions,
warn,
packageInfoCache,
extensions,
mainFields,
preserveSymlinks,
useBrowserOverrides,
baseDir,
moduleDirectories,
rootDir,
ignoreSideEffectsForRoot
});

const resolved =
importeeIsBuiltin && preferBuiltins
? {
packageInfo: undefined,
hasModuleSideEffects: () => null,
hasPackageEntry: true,
packageBrowserField: false
}
: resolvedWithoutBuiltins;
if (!resolved) {
return null;
}
({ preserveSymlinks } = options);
},

const { packageInfo, hasModuleSideEffects, hasPackageEntry, packageBrowserField } = resolved;
let { location } = resolved;
if (packageBrowserField) {
if (Object.prototype.hasOwnProperty.call(packageBrowserField, location)) {
if (!packageBrowserField[location]) {
browserMapCache.set(location, packageBrowserField);
return ES6_BROWSER_EMPTY;
}
location = packageBrowserField[location];
}
browserMapCache.set(location, packageBrowserField);
}
generateBundle() {
readCachedFile.clear();
isFileCached.clear();
isDirCached.clear();
},

if (hasPackageEntry && !preserveSymlinks) {
const fileExists = await exists(location);
if (fileExists) {
location = await realpath(location);
}
async resolveId(importee, importer, opts) {
if (importee === ES6_BROWSER_EMPTY) {
return importee;
}
// ignore IDs with null character, these belong to other plugins
if (/\0/.test(importee)) return null;

idToPackageInfo.set(location, packageInfo);

if (hasPackageEntry) {
if (importeeIsBuiltin && preferBuiltins) {
if (!isPreferBuiltinsSet && resolvedWithoutBuiltins && resolved !== importee) {
this.warn(
`preferring built-in module '${importee}' over local alternative at '${resolvedWithoutBuiltins.location}', pass 'preferBuiltins: false' to disable this behavior or 'preferBuiltins: true' to disable this warning`
);
}
return false;
} else if (jail && location.indexOf(normalize(jail.trim(sep))) !== 0) {
return null;
}
if (/\0/.test(importer)) {
importer = undefined;
}

if (options.modulesOnly && (await exists(location))) {
const code = await readFile(location, 'utf-8');
if (isModule(code)) {
return {
id: `${location}${importSuffix}`,
moduleSideEffects: hasModuleSideEffects(location)
};
const resolved = await doResolveId(this, importee, importer, opts);
if (resolved) {
const resolvedResolved = await this.resolve(resolved.id, importer, { skipSelf: true });
const isExternal = !!(resolvedResolved && resolvedResolved.external);
if (isExternal) {
return false;
}
return null;
}
const result = {
id: `${location}${importSuffix}`,
moduleSideEffects: hasModuleSideEffects(location)
};
return result;
return resolved;
},

load(importee) {
Expand Down
1 change: 1 addition & 0 deletions packages/node-resolve/test/fixtures/resolved-external/main.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import 'external';
Loading

0 comments on commit 3a543df

Please sign in to comment.