From 796c189c99c96f4cc346964e6893af3de54900d0 Mon Sep 17 00:00:00 2001 From: Conrad Buck Date: Thu, 30 May 2019 11:20:50 -0700 Subject: [PATCH] feat(config): allow macro config to come from plugin options in babel config. (#113) * Allow macro config to come from plugin options in babel config. * Respond to review feedback * Ensure failing test output is readable * Ensure cosmiconfig conf is cached * Prioritize file config over plugin options config --- other/docs/author.md | 66 +++++--- src/__tests__/__snapshots__/index.js.snap | 41 +++++ .../config/babel-plugin-macros.config.js | 1 + .../babel-plugin-macros.config.js | 3 + .../fixtures/primitive-config/code.js | 4 + .../primitive-config/configurable.macro.js | 10 ++ src/__tests__/index.js | 159 ++++++++++++++---- src/index.js | 118 +++++++++---- 8 files changed, 310 insertions(+), 92 deletions(-) create mode 100644 src/__tests__/fixtures/primitive-config/babel-plugin-macros.config.js create mode 100644 src/__tests__/fixtures/primitive-config/code.js create mode 100644 src/__tests__/fixtures/primitive-config/configurable.macro.js diff --git a/other/docs/author.md b/other/docs/author.md index e5eb3ec..2e0ad10 100644 --- a/other/docs/author.md +++ b/other/docs/author.md @@ -4,9 +4,9 @@ Is this your first time working with ASTs? Here are some resources: -* [Writing custom Babel and ESLint plugins with ASTs](https://youtu.be/VBscbcm2Mok?list=PLV5CVI1eNcJgNqzNwcs4UKrlJdhfDjshf): A 53 minute talk by [@kentcdodds](https://twitter.com/kentcdodds) -* [babel-handbook](https://github.com/thejameskyle/babel-handbook): A guided handbook on how to use Babel and how to create plugins for Babel by [@thejameskyle](https://twitter.com/thejameskyle) -* [Code Transformation and Linting](https://kentcdodds.com/workshops/#code-transformation-and-linting): A workshop (recording available on Frontend Masters) with exercises of making custom Babel and ESLint plugins +- [Writing custom Babel and ESLint plugins with ASTs](https://youtu.be/VBscbcm2Mok?list=PLV5CVI1eNcJgNqzNwcs4UKrlJdhfDjshf): A 53 minute talk by [@kentcdodds](https://twitter.com/kentcdodds) +- [babel-handbook](https://github.com/thejameskyle/babel-handbook): A guided handbook on how to use Babel and how to create plugins for Babel by [@thejameskyle](https://twitter.com/thejameskyle) +- [Code Transformation and Linting](https://kentcdodds.com/workshops/#code-transformation-and-linting): A workshop (recording available on Frontend Masters) with exercises of making custom Babel and ESLint plugins ## Writing a macro @@ -164,43 +164,53 @@ This is a string used as import declaration's source - i.e. `'./my.macro'`. #### config (EXPERIMENTAL!) -There is an experimental feature that allows users to configure your macro. We -use [`cosmiconfig`][cosmiconfig] to read a `babel-plugin-macros` configuration which +There is an experimental feature that allows users to configure your macro. + +To specify that your plugin is configurable, you pass a `configName` to `createMacro`. + +A configuration is created from data combined from two sources: +We use [`cosmiconfig`][cosmiconfig] to read a `babel-plugin-macros` configuration which can be located in any of the following files up the directories from the importing file: -* `.babel-plugin-macrosrc` -* `.babel-plugin-macrosrc.json` -* `.babel-plugin-macrosrc.yaml` -* `.babel-plugin-macrosrc.yml` -* `.babel-plugin-macrosrc.js` -* `babel-plugin-macros.config.js` -* `babelMacros` in `package.json` +- `.babel-plugin-macrosrc` +- `.babel-plugin-macrosrc.json` +- `.babel-plugin-macrosrc.yaml` +- `.babel-plugin-macrosrc.yml` +- `.babel-plugin-macrosrc.js` +- `babel-plugin-macros.config.js` +- `babelMacros` in `package.json` -To specify that your plugin is configurable, you pass a `configName` to -`createMacro`: +The content of the config will be merged with the content of the babel macros plugin +options. Config options take priority. + +All together specifying and using the config might look like this: ```javascript -const {createMacro} = require('babel-plugin-macros') -const configName = 'taggedTranslations' -module.exports = createMacro(taggedTranslationsMacro, {configName}) -function taggedTranslationsMacro({references, state, babel, config}) { - // config would be taggedTranslations portion of the config as loaded from `cosmiconfig` +// .babel-plugin-macros.config.js +module.exports = { + taggedTranslations: { locale: 'en_US' } } -``` -Then to configure this, users would do something like this: - -```javascript -// babel-plugin-macros.config.js +// .babel.config.js module.exports = { - taggedTranslations: { - someConfig: {}, - }, + plugins: [ + ['macros': { + taggedTranslations: { locale: 'en_GB' } + }] + ] +} + +// taggedTranslations.macro.js +const {createMacro} = require('babel-plugin-macros') +module.exports = createMacro(taggedTranslationsMacro, {configName: 'taggedTranslations'}) +function taggedTranslationsMacro({references, state, babel, config}) { + const { locale = 'en' } = config; } ``` -And the `config` object you would receive would be: `{someConfig: {}}`. +Note that in the above example if both files were sepcified the final locale value would +be `en_US`, since that is the value in the plugin config file. ### Keeping imports diff --git a/src/__tests__/__snapshots__/index.js.snap b/src/__tests__/__snapshots__/index.js.snap index 6095c9f..a76581e 100644 --- a/src/__tests__/__snapshots__/index.js.snap +++ b/src/__tests__/__snapshots__/index.js.snap @@ -221,6 +221,47 @@ global.result = result; `; +exports[`macros when configuration is specified in plugin options: when configuration is specified in plugin options 1`] = ` + +import configured from './configurable.macro' + +// eslint-disable-next-line babel/no-unused-expressions +configured\`stuff\` + + ↓ ↓ ↓ ↓ ↓ ↓ + +// eslint-disable-next-line babel/no-unused-expressions +configured\`stuff\`; + +`; + +exports[`macros when configuration is specified incorrectly in plugin options: when configuration is specified incorrectly in plugin options 1`] = ` + +import configured from './configurable.macro' + +// eslint-disable-next-line babel/no-unused-expressions +configured\`stuff\` + + ↓ ↓ ↓ ↓ ↓ ↓ + +// eslint-disable-next-line babel/no-unused-expressions +configured\`stuff\`; + +`; + +exports[`macros when plugin options configuration cannot be merged with file configuration: when plugin options configuration cannot be merged with file configuration 1`] = ` + +import configured from './configurable.macro' + +// eslint-disable-next-line babel/no-unused-expressions +configured\`stuff\` + + ↓ ↓ ↓ ↓ ↓ ↓ + +Error: /src/__tests__/fixtures/primitive-config/babel-plugin-macros.config.js specified a configurableMacro config of type object, but the the macros plugin's options.configurableMacro did contain an object. Both configs must contain objects for their options to be mergeable. + +`; + exports[`macros when there is an error reading the config, a helpful message is logged 1`] = ` Array [ There was an error trying to load the config "configurableMacro" for the macro imported from "./configurable.macro. Please see the error thrown for more information., diff --git a/src/__tests__/fixtures/config/babel-plugin-macros.config.js b/src/__tests__/fixtures/config/babel-plugin-macros.config.js index 711b575..55dbbe9 100644 --- a/src/__tests__/fixtures/config/babel-plugin-macros.config.js +++ b/src/__tests__/fixtures/config/babel-plugin-macros.config.js @@ -1,5 +1,6 @@ module.exports = { configurableMacro: { + fileConfig: true, someConfig: true, }, } diff --git a/src/__tests__/fixtures/primitive-config/babel-plugin-macros.config.js b/src/__tests__/fixtures/primitive-config/babel-plugin-macros.config.js new file mode 100644 index 0000000..53bfa34 --- /dev/null +++ b/src/__tests__/fixtures/primitive-config/babel-plugin-macros.config.js @@ -0,0 +1,3 @@ +module.exports = { + configurableMacro: 4, +} diff --git a/src/__tests__/fixtures/primitive-config/code.js b/src/__tests__/fixtures/primitive-config/code.js new file mode 100644 index 0000000..9fcb2d0 --- /dev/null +++ b/src/__tests__/fixtures/primitive-config/code.js @@ -0,0 +1,4 @@ +import configured from './configurable.macro' + +// eslint-disable-next-line babel/no-unused-expressions +configured`stuff` diff --git a/src/__tests__/fixtures/primitive-config/configurable.macro.js b/src/__tests__/fixtures/primitive-config/configurable.macro.js new file mode 100644 index 0000000..83b0840 --- /dev/null +++ b/src/__tests__/fixtures/primitive-config/configurable.macro.js @@ -0,0 +1,10 @@ +const {createMacro} = require('../../../../') + +const configName = 'configurableMacro' +const realMacro = jest.fn() +module.exports = createMacro(realMacro, {configName}) +// for testing purposes only +Object.assign(module.exports, { + realMacro, + configName, +}) diff --git a/src/__tests__/index.js b/src/__tests__/index.js index 6624774..b45e99b 100644 --- a/src/__tests__/index.js +++ b/src/__tests__/index.js @@ -8,7 +8,30 @@ import plugin from '../' const projectRoot = path.join(__dirname, '../../') -jest.mock('cosmiconfig', () => jest.fn(require.requireActual('cosmiconfig'))) +jest.mock('cosmiconfig', () => { + const mockSearchSync = jest.fn() + Object.assign(mockSearchSync, { + mockReset() { + return mockSearchSync.mockImplementation( + (filename, configuredCosmiconfig) => + configuredCosmiconfig.searchSync(filename), + ) + }, + }) + + mockSearchSync.mockReset() + + const _cosmiconfigMock = (...args) => ({ + searchSync(filename) { + return mockSearchSync( + filename, + require.requireActual('cosmiconfig')(...args), + ) + }, + }) + + return Object.assign(_cosmiconfigMock, {mockSearchSync}) +}) beforeAll(() => { // copy our mock modules to the node_modules directory @@ -23,6 +46,7 @@ beforeAll(() => { afterEach(() => { // eslint-disable-next-line require('babel-plugin-macros-test-fake/macro').innerFn.mockClear() + cosmiconfigMock.mockSearchSync.mockReset() }) expect.addSnapshotSerializer({ @@ -169,21 +193,26 @@ pluginTester({ fakeMacro('hi') `, teardown() { - // kinda abusing the babel-plugin-tester API here - // to make an extra assertion - // eslint-disable-next-line - const fakeMacro = require('babel-plugin-macros-test-fake/macro') - expect(fakeMacro.innerFn).toHaveBeenCalledTimes(1) - expect(fakeMacro.innerFn).toHaveBeenCalledWith({ - references: expect.any(Object), - source: expect.stringContaining( - 'babel-plugin-macros-test-fake/macro', - ), - state: expect.any(Object), - babel: expect.any(Object), - isBabelMacrosCall: true, - }) - expect(fakeMacro.innerFn.mock.calls[0].babel).toBe(babel) + try { + // kinda abusing the babel-plugin-tester API here + // to make an extra assertion + // eslint-disable-next-line + const fakeMacro = require('babel-plugin-macros-test-fake/macro') + expect(fakeMacro.innerFn).toHaveBeenCalledTimes(1) + expect(fakeMacro.innerFn).toHaveBeenCalledWith({ + references: expect.any(Object), + source: expect.stringContaining( + 'babel-plugin-macros-test-fake/macro', + ), + state: expect.any(Object), + babel: expect.any(Object), + isBabelMacrosCall: true, + }) + expect(fakeMacro.innerFn.mock.calls[0].babel).toBe(babel) + } catch (e) { + console.error(e) + throw e + } }, }, { @@ -258,15 +287,19 @@ pluginTester({ title: 'macros can set their configName and get their config', fixture: path.join(__dirname, 'fixtures/config/code.js'), teardown() { - const babelMacrosConfig = require('./fixtures/config/babel-plugin-macros.config') - const configurableMacro = require('./fixtures/config/configurable.macro') - expect(configurableMacro.realMacro).toHaveBeenCalledTimes(1) - expect(configurableMacro.realMacro).toHaveBeenCalledWith( - expect.objectContaining({ - config: babelMacrosConfig[configurableMacro.configName], - }), - ) - configurableMacro.realMacro.mockClear() + try { + const babelMacrosConfig = require('./fixtures/config/babel-plugin-macros.config') + const configurableMacro = require('./fixtures/config/configurable.macro') + expect(configurableMacro.realMacro).toHaveBeenCalledTimes(1) + expect(configurableMacro.realMacro.mock.calls[0][0].config).toEqual( + babelMacrosConfig[configurableMacro.configName], + ) + + configurableMacro.realMacro.mockClear() + } catch (e) { + console.error(e) + throw e + } }, }, { @@ -275,15 +308,20 @@ pluginTester({ error: true, fixture: path.join(__dirname, 'fixtures/config/code.js'), setup() { - cosmiconfigMock.mockImplementationOnce(() => { + cosmiconfigMock.mockSearchSync.mockImplementationOnce(() => { throw new Error('this is a cosmiconfig error') }) const originalError = console.error console.error = jest.fn() return function teardown() { - expect(console.error).toHaveBeenCalledTimes(1) - expect(console.error.mock.calls[0]).toMatchSnapshot() - console.error = originalError + try { + expect(console.error).toHaveBeenCalledTimes(1) + expect(console.error.mock.calls[0]).toMatchSnapshot() + console.error = originalError + } catch (e) { + console.error(e) + throw e + } } }, }, @@ -291,10 +329,55 @@ pluginTester({ title: 'when there is no config to load, then no config is passed', fixture: path.join(__dirname, 'fixtures/config/code.js'), setup() { - cosmiconfigMock.mockImplementationOnce(() => ({ - searchSync: () => null, - })) + cosmiconfigMock.mockSearchSync.mockImplementationOnce(() => null) return function teardown() { + try { + const configurableMacro = require('./fixtures/config/configurable.macro') + expect(configurableMacro.realMacro).toHaveBeenCalledTimes(1) + expect(configurableMacro.realMacro.mock.calls[0][0].config).toEqual( + {}, + ) + configurableMacro.realMacro.mockClear() + } catch (e) { + console.error(e) + throw e + } + } + }, + }, + { + title: 'when configuration is specified in plugin options', + pluginOptions: { + configurableMacro: { + someConfig: false, + somePluginConfig: true, + }, + }, + fixture: path.join(__dirname, 'fixtures/config/code.js'), + teardown() { + try { + const configurableMacro = require('./fixtures/config/configurable.macro') + expect(configurableMacro.realMacro).toHaveBeenCalledTimes(1) + expect(configurableMacro.realMacro.mock.calls[0][0].config).toEqual({ + fileConfig: true, + someConfig: true, + somePluginConfig: true, + }) + configurableMacro.realMacro.mockClear() + } catch (e) { + console.error(e) + throw e + } + }, + }, + { + title: 'when configuration is specified incorrectly in plugin options', + fixture: path.join(__dirname, 'fixtures/config/code.js'), + pluginOptions: { + configurableMacro: 2, + }, + teardown() { + try { const configurableMacro = require('./fixtures/config/configurable.macro') expect(configurableMacro.realMacro).toHaveBeenCalledTimes(1) expect(configurableMacro.realMacro).not.toHaveBeenCalledWith( @@ -303,9 +386,21 @@ pluginTester({ }), ) configurableMacro.realMacro.mockClear() + } catch (e) { + console.error(e) + throw e } }, }, + { + title: + 'when plugin options configuration cannot be merged with file configuration', + error: true, + fixture: path.join(__dirname, 'fixtures/primitive-config/code.js'), + pluginOptions: { + configurableMacro: {}, + }, + }, { title: 'when a plugin that replaces paths is used, macros still work properly', diff --git a/src/index.js b/src/index.js index 68c2fed..8f5543f 100644 --- a/src/index.js +++ b/src/index.js @@ -18,6 +18,26 @@ class MacroError extends Error { } } +let _configExplorer = null +function getConfigExporer() { + return (_configExplorer = + _configExplorer || + // Lazy load cosmiconfig since it is a relatively large bundle + require('cosmiconfig')('babel-plugin-macros', { + searchPlaces: [ + 'package.json', + '.babel-plugin-macrosrc', + '.babel-plugin-macrosrc.json', + '.babel-plugin-macrosrc.yaml', + '.babel-plugin-macrosrc.yml', + '.babel-plugin-macrosrc.js', + 'babel-plugin-macros.config.js', + ], + packageProp: 'babelMacros', + sync: true, + })) +} + function createMacro(macro, options = {}) { if (options.configName === 'options') { throw new Error( @@ -48,7 +68,7 @@ function nodeResolvePath(source, basedir) { function macrosPlugin( babel, - {require: _require = require, resolvePath = nodeResolvePath} = {}, + {require: _require = require, resolvePath = nodeResolvePath, ...options} = {}, ) { function interopRequire(path) { // eslint-disable-next-line import/no-dynamic-require @@ -88,6 +108,7 @@ function macrosPlugin( babel, interopRequire, resolvePath, + options, }) if (!result || !result.keepImports) { @@ -152,6 +173,7 @@ function applyMacros({ babel, interopRequire, resolvePath, + options, }) { /* istanbul ignore next (pretty much only useful for astexplorer I think) */ const { @@ -183,7 +205,7 @@ function applyMacros({ `Please refer to the documentation to see how to do this properly: https://github.com/kentcdodds/babel-plugin-macros/blob/master/other/docs/author.md#writing-a-macro`, ) } - const config = getConfig(macro, filename, source) + const config = getConfig(macro, filename, source, options) let result try { @@ -228,44 +250,76 @@ function applyMacros({ return result } -// eslint-disable-next-line consistent-return -function getConfig(macro, filename, source) { - if (macro.options.configName) { - try { - // lazy-loading it here to avoid perf issues of loading it up front. - // No I did not measure. Yes I'm a bad person. - // FWIW, this thing told me that cosmiconfig is 227.1 kb of minified JS - // so that's probably significant... https://bundlephobia.com/result?p=cosmiconfig@3.1.0 - // Note that cosmiconfig will cache the babel-plugin-macros config 👍 - const explorer = require('cosmiconfig')('babel-plugin-macros', { - searchPlaces: [ - 'package.json', - `.babel-plugin-macrosrc`, - `.babel-plugin-macrosrc.json`, - `.babel-plugin-macrosrc.yaml`, - `.babel-plugin-macrosrc.yml`, - `.babel-plugin-macrosrc.js`, - `babel-plugin-macros.config.js`, - ], - packageProp: 'babelMacros', - sync: true, - }) - const loaded = explorer.searchSync(filename) - if (loaded) { - return loaded.config[macro.options.configName] +function getConfigFromFile(configName, filename) { + try { + const loaded = getConfigExporer().searchSync(filename) + + if (loaded) { + return { + options: loaded.config[configName], + path: loaded.filepath, } - } catch (error) { + } + } catch (e) { + return {error: e} + } + return {} +} + +function getConfigFromOptions(configName, options) { + if (options.hasOwnProperty(configName)) { + if (options[configName] && typeof options[configName] !== 'object') { // eslint-disable-next-line no-console console.error( - `There was an error trying to load the config "${ - macro.options.configName - }" ` + + `The macro plugin options' ${configName} property was not an object or null.`, + ) + } else { + return {options: options[configName]} + } + } + return {} +} + +function getConfig(macro, filename, source, options) { + const {configName} = macro.options + if (configName) { + const fileConfig = getConfigFromFile(configName, filename) + const optionsConfig = getConfigFromOptions(configName, options) + + if ( + optionsConfig.options === undefined && + fileConfig.options === undefined + ) { + // eslint-disable-next-line no-console + console.error( + `There was an error trying to load the config "${configName}" ` + `for the macro imported from "${source}. ` + `Please see the error thrown for more information.`, ) - throw error + if (fileConfig.error !== undefined) { + throw fileConfig.error + } + } + + if ( + fileConfig.options !== undefined && + optionsConfig.options !== undefined && + typeof fileConfig.options !== 'object' + ) { + throw new Error( + `${fileConfig.path} specified a ${configName} config of type ` + + `${typeof optionsConfig.options}, but the the macros plugin's ` + + `options.${configName} did contain an object. Both configs must ` + + `contain objects for their options to be mergeable.`, + ) + } + + return { + ...optionsConfig.options, + ...fileConfig.options, } } + return undefined } /*