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

Break infinite loops in this.resolve #4000

Merged
merged 5 commits into from
Mar 19, 2021
Merged
Show file tree
Hide file tree
Changes from 4 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
18 changes: 10 additions & 8 deletions browser/resolveId.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,23 @@
import { CustomPluginOptions } from '../src/rollup/types';
import { CustomPluginOptions, Plugin, ResolvedId } from '../src/rollup/types';
import { PluginDriver } from '../src/utils/PluginDriver';
import { resolveIdViaPlugins } from '../src/utils/resolveIdViaPlugins';
import { throwNoFileSystem } from './error';

export async function resolveId(
source: string,
importer: string | undefined,
_preserveSymlinks: boolean,
pluginDriver: PluginDriver,
skip: number | null,
moduleLoaderResolveId: (
source: string,
importer: string | undefined,
customOptions: CustomPluginOptions | undefined,
skip: { importer: string | undefined; plugin: Plugin; source: string }[] | null
) => Promise<ResolvedId | null>,
skip: { importer: string | undefined; plugin: Plugin; source: string }[] | null,
customOptions: CustomPluginOptions | undefined
) {
const pluginResult = await pluginDriver.hookFirst(
'resolveId',
[source, importer, { custom: customOptions }],
null,
skip
);
const pluginResult = await resolveIdViaPlugins(source, importer,pluginDriver, moduleLoaderResolveId, skip, customOptions);
if (pluginResult == null) {
throwNoFileSystem('path.resolve');
}
Expand Down
2 changes: 1 addition & 1 deletion docs/05-plugin-development.md
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,7 @@ Use Rollup's internal acorn instance to parse code to an AST.
#### `this.resolve(source: string, importer?: string, options?: {skipSelf?: boolean, custom?: {[plugin: string]: any}}) => Promise<{id: string, external: boolean, moduleSideEffects: boolean | 'no-treeshake', syntheticNamedExports: boolean | string, meta: {[plugin: string]: any}} | null>`
Resolve imports to module ids (i.e. file names) using the same plugins that Rollup uses, and determine if an import should be external. If `null` is returned, the import could not be resolved by Rollup or any plugin but was not explicitly marked as external by the user.

If you pass `skipSelf: true`, then the `resolveId` hook of the plugin from which `this.resolve` is called will be skipped when resolving.
If you pass `skipSelf: true`, then the `resolveId` hook of the plugin from which `this.resolve` is called will be skipped when resolving. When other plugins themselves also call `this.resolve` in their `resolveId` hooks with the *exact same `source` and `importer`* while handling the original `this.resolve` call, then the `resolveId` hook of the original plugin will be skipped for those calls as well. The rationale here is that the plugin already stated that it "does not know" how to resolve this particular combination of `source` and `importer` at this point in time. If you do not want this behaviour, do not use `skipSelf` but implement your own infinite loop prevention mechanism if necessary.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 still pretty confusing, but I'm not sure how it could be any clearer 😆

Does feel like it fits the responsibility of skipSelf.


You can also pass an object of plugin-specific options via the `custom` option, see [custom resolver options](guide/en/#custom-resolver-options) for details.

Expand Down
9 changes: 6 additions & 3 deletions src/ModuleLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
HasModuleSideEffects,
NormalizedInputOptions,
PartialResolvedId,
Plugin,
ResolvedId,
ResolveIdResult,
SourceDescription
Expand Down Expand Up @@ -135,12 +136,12 @@ export class ModuleLoader {
return module;
}

async resolveId(
resolveId = async (
source: string,
importer: string | undefined,
customOptions: CustomPluginOptions | undefined,
skip: number | null = null
): Promise<ResolvedId | null> {
skip: { importer: string | undefined; plugin: Plugin; source: string }[] | null = null
): Promise<ResolvedId | null> => {
return this.addDefaultsToResolvedId(
this.getNormalizedResolvedIdWithoutDefaults(
this.options.external(source, importer, false)
Expand All @@ -150,6 +151,7 @@ export class ModuleLoader {
importer,
this.options.preserveSymlinks,
this.pluginDriver,
this.resolveId,
skip,
customOptions
),
Expand Down Expand Up @@ -444,6 +446,7 @@ export class ModuleLoader {
importer,
this.options.preserveSymlinks,
this.pluginDriver,
this.resolveId,
null,
EMPTY_OBJECT
);
Expand Down
212 changes: 108 additions & 104 deletions src/utils/PluginContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,121 +44,126 @@ function getDeprecatedContextHandler<H extends Function>(
}) as unknown) as H;
}

export function getPluginContexts(
export function getPluginContext(
plugin: Plugin,
pluginCache: Record<string, SerializablePluginCache> | void,
graph: Graph,
options: NormalizedInputOptions,
fileEmitter: FileEmitter
): (plugin: Plugin, pluginIndex: number) => PluginContext {
const existingPluginNames = new Set<string>();
return (plugin, pidx) => {
let cacheable = true;
if (typeof plugin.cacheKey !== 'string') {
if (
plugin.name.startsWith(ANONYMOUS_PLUGIN_PREFIX) ||
plugin.name.startsWith(ANONYMOUS_OUTPUT_PLUGIN_PREFIX) ||
existingPluginNames.has(plugin.name)
) {
cacheable = false;
} else {
existingPluginNames.add(plugin.name);
}
}

let cacheInstance: PluginCache;
if (!pluginCache) {
cacheInstance = NO_CACHE;
} else if (cacheable) {
const cacheKey = plugin.cacheKey || plugin.name;
cacheInstance = createPluginCache(
pluginCache[cacheKey] || (pluginCache[cacheKey] = Object.create(null))
);
fileEmitter: FileEmitter,
existingPluginNames: Set<string>
): PluginContext {
let cacheable = true;
if (typeof plugin.cacheKey !== 'string') {
if (
plugin.name.startsWith(ANONYMOUS_PLUGIN_PREFIX) ||
plugin.name.startsWith(ANONYMOUS_OUTPUT_PLUGIN_PREFIX) ||
existingPluginNames.has(plugin.name)
) {
cacheable = false;
} else {
cacheInstance = getCacheForUncacheablePlugin(plugin.name);
existingPluginNames.add(plugin.name);
}
}

const context: PluginContext = {
addWatchFile(id) {
if (graph.phase >= BuildPhase.GENERATE) {
return this.error(errInvalidRollupPhaseForAddWatchFile());
}
graph.watchFiles[id] = true;
},
cache: cacheInstance,
emitAsset: getDeprecatedContextHandler(
(name: string, source?: string | Uint8Array) =>
fileEmitter.emitFile({ type: 'asset', name, source }),
'emitAsset',
'emitFile',
plugin.name,
true,
options
),
emitChunk: getDeprecatedContextHandler(
(id: string, options?: { name?: string }) =>
fileEmitter.emitFile({ type: 'chunk', id, name: options && options.name }),
'emitChunk',
'emitFile',
plugin.name,
true,
options
),
emitFile: fileEmitter.emitFile,
error(err): never {
return throwPluginError(err, plugin.name);
},
getAssetFileName: getDeprecatedContextHandler(
fileEmitter.getFileName,
'getAssetFileName',
'getFileName',
plugin.name,
true,
options
),
getChunkFileName: getDeprecatedContextHandler(
fileEmitter.getFileName,
'getChunkFileName',
'getFileName',
plugin.name,
true,
options
),
getFileName: fileEmitter.getFileName,
getModuleIds: () => graph.modulesById.keys(),
getModuleInfo: graph.getModuleInfo,
getWatchFiles: () => Object.keys(graph.watchFiles),
isExternal: getDeprecatedContextHandler(
(id: string, parentId: string | undefined, isResolved = false) =>
options.external(id, parentId, isResolved),
'isExternal',
'resolve',
plugin.name,
true,
options
),
meta: {
rollupVersion,
watchMode: graph.watchMode
},
get moduleIds() {
function* wrappedModuleIds() {
warnDeprecation(
{
message: `Accessing "this.moduleIds" on the plugin context by plugin ${plugin.name} is deprecated. The "this.getModuleIds" plugin context function should be used instead.`,
plugin: plugin.name
},
false,
options
);
yield* moduleIds;
}
let cacheInstance: PluginCache;
if (!pluginCache) {
cacheInstance = NO_CACHE;
} else if (cacheable) {
const cacheKey = plugin.cacheKey || plugin.name;
cacheInstance = createPluginCache(
pluginCache[cacheKey] || (pluginCache[cacheKey] = Object.create(null))
);
} else {
cacheInstance = getCacheForUncacheablePlugin(plugin.name);
}

const context: PluginContext = {
addWatchFile(id) {
if (graph.phase >= BuildPhase.GENERATE) {
return this.error(errInvalidRollupPhaseForAddWatchFile());
}
graph.watchFiles[id] = true;
},
cache: cacheInstance,
emitAsset: getDeprecatedContextHandler(
(name: string, source?: string | Uint8Array) =>
fileEmitter.emitFile({ type: 'asset', name, source }),
'emitAsset',
'emitFile',
plugin.name,
true,
options
),
emitChunk: getDeprecatedContextHandler(
(id: string, options?: { name?: string }) =>
fileEmitter.emitFile({ type: 'chunk', id, name: options && options.name }),
'emitChunk',
'emitFile',
plugin.name,
true,
options
),
emitFile: fileEmitter.emitFile,
error(err): never {
return throwPluginError(err, plugin.name);
},
getAssetFileName: getDeprecatedContextHandler(
fileEmitter.getFileName,
'getAssetFileName',
'getFileName',
plugin.name,
true,
options
),
getChunkFileName: getDeprecatedContextHandler(
fileEmitter.getFileName,
'getChunkFileName',
'getFileName',
plugin.name,
true,
options
),
getFileName: fileEmitter.getFileName,
getModuleIds: () => graph.modulesById.keys(),
getModuleInfo: graph.getModuleInfo,
getWatchFiles: () => Object.keys(graph.watchFiles),
isExternal: getDeprecatedContextHandler(
(id: string, parentId: string | undefined, isResolved = false) =>
options.external(id, parentId, isResolved),
'isExternal',
'resolve',
plugin.name,
true,
options
),
meta: {
rollupVersion,
watchMode: graph.watchMode
},
get moduleIds() {
function* wrappedModuleIds() {
warnDeprecation(
{
message: `Accessing "this.moduleIds" on the plugin context by plugin ${plugin.name} is deprecated. The "this.getModuleIds" plugin context function should be used instead.`,
plugin: plugin.name
},
false,
options
);
yield* moduleIds;
}

const moduleIds = graph.modulesById.keys();
return wrappedModuleIds();
},
parse: graph.contextParse.bind(graph),
resolve(source, importer, { custom, skipSelf } = BLANK) {
return graph.moduleLoader.resolveId(source, importer, custom, skipSelf ? pidx : null);
return graph.moduleLoader.resolveId(
source,
importer,
custom,
skipSelf ? [{ importer, plugin, source }] : null
);
},
resolveId: getDeprecatedContextHandler(
(source: string, importer: string | undefined) =>
Expand All @@ -181,5 +186,4 @@ export function getPluginContexts(
}
};
return context;
};
}