Skip to content

Commit

Permalink
Respond to review feedback. Actually cache configs.
Browse files Browse the repository at this point in the history
  • Loading branch information
conartist6 committed May 29, 2019
1 parent 65ed764 commit 1a6cae1
Show file tree
Hide file tree
Showing 3 changed files with 156 additions and 104 deletions.
2 changes: 1 addition & 1 deletion src/__tests__/__snapshots__/index.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ configured\`stuff\`
↓ ↓ ↓ ↓ ↓ ↓
Error: <PROJECT_ROOT>/src/__tests__/fixtures/primitive-config/babel-plugin-macros.config.js specified a configurableMacro config of type number, but the the macros plugin's options.configurableMacro did contain an object.Both configs must contain objects for their options to be mergeable.
Error: <PROJECT_ROOT>/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.
`;
Expand Down
151 changes: 97 additions & 54 deletions src/__tests__/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -23,6 +46,7 @@ beforeAll(() => {
afterEach(() => {
// eslint-disable-next-line
require('babel-plugin-macros-test-fake/macro').innerFn.mockClear()
cosmiconfigMock.mockSearchSync.mockReset()
})

expect.addSnapshotSerializer({
Expand Down Expand Up @@ -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
}
},
},
{
Expand Down Expand Up @@ -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
}
},
},
{
Expand All @@ -275,34 +308,40 @@ 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
}
}
},
},
{
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() {
const configurableMacro = require('./fixtures/config/configurable.macro')
expect(configurableMacro.realMacro).toHaveBeenCalledTimes(1)
expect(configurableMacro.realMacro).not.toHaveBeenCalledWith(
expect.objectContaining({
config: expect.any,
}),
)
configurableMacro.realMacro.mockClear()
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
}
}
},
},
Expand All @@ -316,18 +355,19 @@ pluginTester({
},
fixture: path.join(__dirname, 'fixtures/config/code.js'),
teardown() {
const configurableMacro = require('./fixtures/config/configurable.macro')
expect(configurableMacro.realMacro).toHaveBeenCalledTimes(1)
expect(configurableMacro.realMacro).toHaveBeenCalledWith(
expect.objectContaining({
config: {
fileConfig: true,
someConfig: false,
somePluginConfig: true,
},
}),
)
configurableMacro.realMacro.mockClear()
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: false,
somePluginConfig: true,
})
configurableMacro.realMacro.mockClear()
} catch (e) {
console.error(e)
throw e
}
},
},
{
Expand All @@ -336,8 +376,8 @@ pluginTester({
pluginOptions: {
configurableMacro: 2,
},
setup() {
return function teardown() {
teardown() {
try {
const configurableMacro = require('./fixtures/config/configurable.macro')
expect(configurableMacro.realMacro).toHaveBeenCalledTimes(1)
expect(configurableMacro.realMacro).not.toHaveBeenCalledWith(
Expand All @@ -346,6 +386,9 @@ pluginTester({
}),
)
configurableMacro.realMacro.mockClear()
} catch (e) {
console.error(e)
throw e
}
},
},
Expand Down
107 changes: 58 additions & 49 deletions src/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,18 @@
const p = require('path')
const resolve = require('resolve')
const configExplorer = 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 printAST = require('ast-pretty-print')

const macrosRegex = /[./]macro(\.js)?$/
Expand Down Expand Up @@ -230,77 +243,73 @@ function applyMacros({
return result
}

function getConfig(macro, filename, source, options) {
const {configName} = macro.options
if (configName) {
let callOptions, configOptions, configError, configPath
function getConfigFromFile(configName, filename) {
try {
const loaded = configExplorer.searchSync(filename)

if (Object.prototype.hasOwnProperty.call(options, configName)) {
if (options[configName] && typeof options[configName] !== 'object') {
// eslint-disable-next-line no-console
console.error(
`The macro plugin options' ${configName} property was not an object or null.`,
)
} else {
callOptions = options[configName]
if (loaded) {
return {
options: loaded.config[configName],
path: loaded.filepath,
}
}
} catch (e) {
return {error: e}
}
return {}
}

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/[email protected]
// 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) {
configOptions = loaded.config[configName]
configPath = loaded.filepath
}
} catch (e) {
configError = e
function getConfigFromOptions(configName, options) {
if (Object.prototype.hasOwnProperty.call(options, configName)) {
if (options[configName] && typeof options[configName] !== 'object') {
// eslint-disable-next-line no-console
console.error(
`The macro plugin options' ${configName} property was not an object or null.`,
)
} else {
return {options: options[configName]}
}
}
return {}
}

if (callOptions === undefined && configOptions === undefined) {
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.`,
)
if (configError !== undefined) {
throw configError
if (fileConfig.error !== undefined) {
throw fileConfig.error
}
}

if (
configOptions &&
callOptions !== undefined &&
typeof configOptions !== 'object'
fileConfig.options !== undefined &&
optionsConfig.options !== undefined &&
typeof fileConfig.options !== 'object'
) {
throw new Error(
`${configPath} specified a ${configName} config of type ${typeof configOptions}, ` +
`but the the macros plugin's options.${configName} did contain an object.` +
`Both configs must contain objects for their options to be mergeable.`,
`${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 {
...configOptions,
...callOptions,
...fileConfig.options,
...optionsConfig.options,
}
}
return undefined
Expand Down

0 comments on commit 1a6cae1

Please sign in to comment.