Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/rollup/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,14 @@ export interface PluginContext {
getAssetFileName: (assetId: string) => string;
warn(warning: RollupWarning | string, pos?: { line: number; column: number }): void;
error(err: RollupError | string, pos?: { line: number; column: number }): void;
moduleIds: IterableIterator<string>;
getModuleInfo: (
moduleId: string
) => {
id: string;
isExternal: boolean;
importedIds: string[];
};
}

export interface PluginContextMeta {
Expand Down
16 changes: 16 additions & 0 deletions src/utils/pluginDriver.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { version as rollupVersion } from 'package.json';
import Graph from '../Graph';
import Module from '../Module';
import {
InputOptions,
Plugin,
Expand Down Expand Up @@ -114,6 +115,21 @@ export function createPluginDriver(
warning.plugin = plugin.name || '(anonymous plugin)';
graph.warn(warning);
},
moduleIds: graph.moduleById.keys(),
getModuleInfo: (moduleId: string) => {
const foundModule = graph.moduleById.get(moduleId);
if (foundModule == null) {
throw new Error(`Unable to find module ${moduleId}`);
}

return {
id: foundModule.id,
isExternal: !!foundModule.isExternal,
importedIds: foundModule.isExternal
? []
: (foundModule as Module).sources.map(id => (foundModule as Module).resolvedIds[id])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What happens if this is called before the module has been resolved? Is that possible?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For now, moduleIds will be empty and getModuleInfo will throw a "module not found" error. Added a test to make sure this is the case. This may or may not be what users want but if we e.g. do not provide this information on certain hooks at all, I fear this is something that is hard to maintain properly when e.g. new hooks are added. Thus here you get a snapshot of what the current state of the graph is.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for clarifying. It might be nice to separate the graph into two states then - resolved and unresolved, and for this function to always throw an early error when called when the graph is partially unresolved so that we aren't exposing the partial state.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good point but is this really an issue or more a way to avoid people relying on unreliable information? But having an internal flag marking the graph as incomplete would definitely be a future-proof way of handling this. Created #2598 to track. There may be more opinions on this issue.

};
},

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Very nice, yes, that is indeed very maintainable now!
Some notes that might stream-line this even more:

  • I really like the idea of exposing the modules as an iterator!
  • My feeling is we could just move this API to the top level as the way you designed it, I think there will not be much need to extend it except in what is returned by getModuleInfo. I.e. I would just put getModuleInfo directly on the context.
  • allModules could be come top-level as well but I am not 100% sold on the name as technically, this is providing ids and I think the all might be obvious and the name could reflect better that it is a getter. Maybe getModuleIds?

watcher
};
return context;
Expand Down
6 changes: 4 additions & 2 deletions test/form/samples/transform-bundle-plugin-options/_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ module.exports = {
plugins: [
{
transformBundle(code, options) {
console.log(Object.keys(options));
assert.strictEqual(Object.keys(options).join(', '), require('../../../misc/optionList').output);
assert.strictEqual(
Object.keys(options).join(', '),
require('../../../misc/optionList').output
);
return options.format;
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
const assert = require('assert');
const path = require('path');

const ID_MAIN = path.join(__dirname, 'main.js');

module.exports = {
description: 'handles accessing module information via plugins early in a graceful way',
options: {
external: ['path'],
plugins: [
{
buildStart() {
assert.deepEqual(Array.from(this.moduleIds), []);
// should throw "not found" error
this.getModuleInfo(ID_MAIN);
}
}
]
},
error: {
code: 'PLUGIN_ERROR',
hook: 'buildStart',
message: `Unable to find module ${ID_MAIN}`,
plugin: 'Plugin at pos 0'
}
};
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default 42;
47 changes: 47 additions & 0 deletions test/function/samples/plugin-module-information/_config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
const assert = require('assert');
const path = require('path');

const ID_MAIN = path.join(__dirname, 'main.js');
const ID_FOO = path.join(__dirname, 'foo.js');
const ID_NESTED = path.join(__dirname, 'nested', 'nested.js');
const ID_PATH = 'path';

let rendered = false;

module.exports = {
description: 'provides module information on the plugin context',
options: {
external: ['path'],
plugins: [
{
renderStart() {
rendered = true;
assert.deepEqual(Array.from(this.moduleIds), [ID_MAIN, ID_FOO, ID_NESTED, ID_PATH]);
assert.deepEqual(this.getModuleInfo(ID_MAIN), {
id: ID_MAIN,
importedIds: [ID_FOO, ID_NESTED],
isExternal: false
});
assert.deepEqual(this.getModuleInfo(ID_FOO), {
id: ID_FOO,
importedIds: [ID_PATH],
isExternal: false
});
assert.deepEqual(this.getModuleInfo(ID_NESTED), {
id: ID_NESTED,
importedIds: [ID_FOO],
isExternal: false
});
assert.deepEqual(this.getModuleInfo(ID_PATH), {
id: ID_PATH,
importedIds: [],
isExternal: true
});
}
}
]
},
bundle() {
assert.ok(rendered);
}
};
3 changes: 3 additions & 0 deletions test/function/samples/plugin-module-information/foo.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import path from 'path';

export const foo = path.resolve('foo');
4 changes: 4 additions & 0 deletions test/function/samples/plugin-module-information/main.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export {foo} from './foo.js';
import { nested } from './nested/nested';

export {nested};
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { foo } from '../foo.js';

export const nested = 'nested' + foo;