From 8ec00a680c7de91977fc43595460170b5f1e649c Mon Sep 17 00:00:00 2001 From: Brian Donovan Date: Wed, 29 Mar 2017 17:42:40 -0700 Subject: [PATCH] fix: ensure plugin option association (#9) Previously the basename of the resolved plugin file would be used as the plugin name when applying options. However, that meant that if the file was called "index.js" in the plugin package, i.e. named "my-codemod", it was distributed with, the plugin options would have to be given to a plugin named "index" rather than "my-codemod". --- src/Options.ts | 104 ++++++++++++++++++++++++++-------- src/TransformRunner.ts | 8 +-- src/index.ts | 2 +- test/OptionsTest.ts | 48 ++++++++++++++-- test/fixtures/plugin/index.js | 6 ++ 5 files changed, 132 insertions(+), 36 deletions(-) create mode 100644 test/fixtures/plugin/index.js diff --git a/src/Options.ts b/src/Options.ts index 25669165..41624536 100644 --- a/src/Options.ts +++ b/src/Options.ts @@ -1,19 +1,53 @@ +import * as Babel from 'babel-core'; import { existsSync, readFileSync } from 'fs'; import { hasMagic as hasGlob, sync as globSync } from 'glob'; import { basename, extname, resolve } from 'path'; import { sync as resolveSync } from 'resolve'; import { PathPredicate } from './iterateSources'; -import { Plugin } from './TransformRunner'; +import { BabelPlugin, RawBabelPlugin } from './TransformRunner'; export const DEFAULT_EXTENSIONS = new Set(['.js', '.jsx']); export type ParseOptionsResult = Options | Error; -export default class Options { - private plugins?: Array; +export class Plugin { + readonly declaredName?: string; + constructor( + readonly rawPlugin: RawBabelPlugin, + readonly inferredName: string, + readonly path?: string, + ) { + let instance = rawPlugin(Babel); + + if (instance.name) { + this.declaredName = instance.name; + } + } + + static load(path: string, inferredName: string) { + let exports = require(path); + let plugin; + + if (exports.default) { + plugin = exports.default; + } else { + plugin = exports; + } + + let rawPlugin = plugin; + + return new Plugin( + rawPlugin, + inferredName, + path + ); + } +} + +export default class Options { constructor( readonly sourcePaths: Array, - readonly pluginFilePaths: Array, + readonly plugins: Array, readonly pluginOptions: Map, readonly extensions: Set, readonly requires: Array, @@ -23,10 +57,6 @@ export default class Options { ) {} getPlugins(): Array { - if (!this.plugins) { - this.plugins = this.loadPlugins(); - } - return this.plugins; } @@ -36,30 +66,53 @@ export default class Options { } } - private loadPlugins(): Array { - return this.pluginFilePaths.map(pluginFilePath => { - let name = basename(pluginFilePath, extname(pluginFilePath)); - let options = this.pluginOptions.get(name); - let exports = require(pluginFilePath); - let plugin; - - if (exports.default) { - plugin = exports.default; - } else { - plugin = exports; + getPlugin(name: string): Plugin | null { + for (let plugin of this.plugins) { + if (plugin.declaredName === name || plugin.inferredName === name) { + return plugin; } + } + + return null; + } + + getBabelPlugins(): Array { + let result: Array = []; + + for (let plugin of this.plugins) { + let options = plugin.declaredName && + this.pluginOptions.get(plugin.declaredName) || + this.pluginOptions.get(plugin.inferredName); if (options) { - return [plugin, options]; + result.push([plugin.rawPlugin, options]); } else { - return plugin; + result.push(plugin.rawPlugin); } - }); + } + + return result; + } + + getBabelPlugin(name: string): BabelPlugin | null { + let plugin = this.getPlugin(name); + + if (!plugin) { + return null; + } + + let options = this.pluginOptions.get(name); + + if (options) { + return [plugin.rawPlugin, options]; + } else { + return plugin.rawPlugin; + } } static parse(args: Array): ParseOptionsResult { let sourcePaths: Array = []; - let pluginFilePaths: Array = []; + let plugins: Array = []; let pluginOptions: Map = new Map(); let extensions = DEFAULT_EXTENSIONS; let ignore = (path: string, basename: string, root: string) => basename[0] === '.'; @@ -74,7 +127,8 @@ export default class Options { case '-p': case '--plugin': i++; - pluginFilePaths.push(getRequirableModulePath(args[i])); + let path = args[i]; + plugins.push(Plugin.load(getRequirableModulePath(path), basename(path, extname(path)))); break; case '-o': @@ -137,7 +191,7 @@ export default class Options { return new Options( sourcePaths, - pluginFilePaths, + plugins, pluginOptions, extensions, requires, diff --git a/src/TransformRunner.ts b/src/TransformRunner.ts index 89934075..1a8e60db 100644 --- a/src/TransformRunner.ts +++ b/src/TransformRunner.ts @@ -19,9 +19,9 @@ export class SourceTransformResult { ) {} } -export type Plugin = - ((babel: typeof Babel) => { visitor: Visitor }) | - [(babel: typeof Babel) => { visitor: Visitor }, object]; +export type RawBabelPlugin = (babel: typeof Babel) => { name?: string, visitor: Visitor }; +export type RawBabelPluginWithOptions = [RawBabelPlugin, object]; +export type BabelPlugin = RawBabelPlugin | RawBabelPluginWithOptions; export type TransformRunnerDelegate = { transformStart?: (runner: TransformRunner) => void; @@ -33,7 +33,7 @@ export type TransformRunnerDelegate = { export default class TransformRunner { constructor( readonly sources: IterableIterator | Array, - readonly plugins: Array, + readonly plugins: Array, private readonly delegate: TransformRunnerDelegate = {}, ) {} diff --git a/src/index.ts b/src/index.ts index cdf6e57b..a5f38683 100644 --- a/src/index.ts +++ b/src/index.ts @@ -65,7 +65,7 @@ export default async function run(args: Array) { options.loadRequires(); - let plugins = options.getPlugins(); + let plugins = options.getBabelPlugins(); let runner: TransformRunner; if (options.stdio) { diff --git a/test/OptionsTest.ts b/test/OptionsTest.ts index 79aaa307..b3c31e39 100644 --- a/test/OptionsTest.ts +++ b/test/OptionsTest.ts @@ -1,11 +1,12 @@ import { deepEqual, strictEqual } from 'assert'; +import { inspect } from 'util'; import Options, { ParseOptionsResult } from '../src/Options'; describe('Options', function() { it('has sensible defaults', function() { let options = assertOptionsParsed(Options.parse([])); deepEqual(options.extensions, new Set(['.js', '.jsx'])); - deepEqual(options.pluginFilePaths, []); + deepEqual(options.plugins, []); deepEqual(options.sourcePaths, []); deepEqual(options.requires, []); strictEqual(options.pluginOptions.size, 0); @@ -22,11 +23,6 @@ describe('Options', function() { strictEqual(error.message, 'unexpected option: --wtf'); }); - it('allows existing file paths as plugins', function() { - let options = assertOptionsParsed(Options.parse(['--plugin', __filename])); - deepEqual(options.pluginFilePaths, [__filename]); - }); - it('interprets non-option arguments as paths', function() { let options = assertOptionsParsed(Options.parse(['src/', 'a.js'])); deepEqual(options.sourcePaths, ['src/', 'a.js']); @@ -47,6 +43,46 @@ describe('Options', function() { deepEqual(options.pluginOptions.get('my-plugin'), { foo: true }); }); + it('associates plugin options based on declared name', function() { + let options = assertOptionsParsed(Options.parse([ + '--plugin', + './test/fixtures/plugin/index.js', + '--plugin-options', + 'basic-plugin={"a": true}' + ])); + + // "basic-plugin" is declared in the plugin file + deepEqual(options.pluginOptions.get('basic-plugin'), { a: true }); + + let babelPlugin = options.getBabelPlugin('basic-plugin'); + + if (!Array.isArray(babelPlugin)) { + throw new Error(`expected plugin to be [plugin, options] tuple: ${inspect(babelPlugin)}`); + } + + deepEqual(babelPlugin[1], { a: true }); + }); + + it('associates plugin options based on inferred name', function() { + let options = assertOptionsParsed(Options.parse([ + '--plugin', + './test/fixtures/plugin/index.js', + '--plugin-options', + 'index={"a": true}' + ])); + + // "index" is the name of the file + deepEqual(options.pluginOptions.get('index'), { a: true }); + + let babelPlugin = options.getBabelPlugin('index'); + + if (!Array.isArray(babelPlugin)) { + throw new Error(`expected plugin to be [plugin, options] tuple: ${inspect(babelPlugin)}`); + } + + deepEqual(babelPlugin[1], { a: true }); + }); + it('can parse a JSON file for plugin options', function() { // You wouldn't actually use package.json, but it's a convenient JSON file. let options = assertOptionsParsed(Options.parse(['-o', 'my-plugin=@package.json'])); diff --git a/test/fixtures/plugin/index.js b/test/fixtures/plugin/index.js new file mode 100644 index 00000000..758edfce --- /dev/null +++ b/test/fixtures/plugin/index.js @@ -0,0 +1,6 @@ +module.exports = function() { + return { + name: 'basic-plugin', + visitor: {} + } +};