Skip to content

Commit

Permalink
fix(options): merge options instead of replace them
Browse files Browse the repository at this point in the history
resolves #12
  • Loading branch information
kwelch committed Jul 26, 2017
1 parent 34aaea0 commit 70e987b
Show file tree
Hide file tree
Showing 7 changed files with 265 additions and 23 deletions.
10 changes: 10 additions & 0 deletions src/__snapshots__/index.spec.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -225,3 +225,13 @@ function logAll() {
}
"
`;

exports[`12. Merge options - fix #12 1`] = `
"
console.log(a);
↓ ↓ ↓ ↓ ↓ ↓
console.log(\\"unknown(1:0)\\", \\"a\\", a);
"
`;
27 changes: 4 additions & 23 deletions src/index.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,4 @@
const defaultSettings = {
injectScope: true,
injectVariableName: true,
injectFileName: true,
};

const defaultMethods = ['debug', 'error', 'exception', 'info', 'log', 'warn'];
import { buildOptions } from './utils/pluginOptions';

const idNameSelector = path => path.node.id.name;
const keyNameSelector = path => path.node.key.name;
Expand Down Expand Up @@ -35,15 +29,16 @@ export default function({ types: t }) {
if (!looksLike(path.node, { name: 'console' })) {
return;
}
const settings = buildSettings(opts || {});
// find somewhere we can move this so that it only needs to be called once.
const settings = buildOptions(opts || {});
const parentCallExp = path.findParent(t.isCallExpression);
if (isTrackingConsoleCallStatement(path, parentCallExp, settings)) {
callExpressions.add(parentCallExp);
}
},
Program: {
exit(_, { file, opts }) {
const settings = buildSettings(opts || {});
const settings = buildOptions(opts || {});
callExpressions.forEach(callExp => {
if (!callExp || evaluatedExpressions.has(callExp)) {
return;
Expand Down Expand Up @@ -100,20 +95,6 @@ export default function({ types: t }) {
}, []);
}

function buildSettings(opts) {
// remove ignore patterns from settings since it has been consumed already
// eslint-disable-next-line no-unused-vars
const { methods, ignorePatterns, ...flags } = opts;
// output spreads the flags over each method
// in the future this could be expanded to allow method level config
return (methods || defaultMethods).reduce((acc, curr) => {
return {
...acc,
[curr]: Object.values(flags).length ? flags : defaultSettings,
};
}, {});
}

function findCallScope(path, scope = []) {
const parentFunc = path.findParent(path =>
Object.keys(scopeHandlers).includes(path.type)
Expand Down
5 changes: 5 additions & 0 deletions src/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,5 +81,10 @@ pluginTester({
methods: ['debug', 'log', 'info'],
},
},
{
title: 'Merge options - fix #12',
code: `console.log(a);`,
pluginOptions: { injectScope: false },
},
],
});
186 changes: 186 additions & 0 deletions src/utils/__snapshots__/pluginOptions.spec.js.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`buildOptions should allow methods to be changed 1`] = `
Object {
"debug": Object {
"injectFileName": true,
"injectScope": true,
"injectVariableName": true,
},
}
`;

exports[`buildOptions should overwrite defaults 1`] = `
Object {
"debug": Object {
"injectFileName": false,
"injectScope": true,
"injectVariableName": true,
},
"error": Object {
"injectFileName": false,
"injectScope": true,
"injectVariableName": true,
},
"exception": Object {
"injectFileName": false,
"injectScope": true,
"injectVariableName": true,
},
"info": Object {
"injectFileName": false,
"injectScope": true,
"injectVariableName": true,
},
"log": Object {
"injectFileName": false,
"injectScope": true,
"injectVariableName": true,
},
"warn": Object {
"injectFileName": false,
"injectScope": true,
"injectVariableName": true,
},
}
`;

exports[`buildOptions should return defaults for invalid args 1`] = `
Object {
"debug": Object {
"injectFileName": true,
"injectScope": true,
"injectVariableName": true,
},
"error": Object {
"injectFileName": true,
"injectScope": true,
"injectVariableName": true,
},
"exception": Object {
"injectFileName": true,
"injectScope": true,
"injectVariableName": true,
},
"info": Object {
"injectFileName": true,
"injectScope": true,
"injectVariableName": true,
},
"log": Object {
"injectFileName": true,
"injectScope": true,
"injectVariableName": true,
},
"warn": Object {
"injectFileName": true,
"injectScope": true,
"injectVariableName": true,
},
}
`;

exports[`buildOptions should return defaults for invalid args 2`] = `
Object {
"debug": Object {
"injectFileName": true,
"injectScope": true,
"injectVariableName": true,
},
"error": Object {
"injectFileName": true,
"injectScope": true,
"injectVariableName": true,
},
"exception": Object {
"injectFileName": true,
"injectScope": true,
"injectVariableName": true,
},
"info": Object {
"injectFileName": true,
"injectScope": true,
"injectVariableName": true,
},
"log": Object {
"injectFileName": true,
"injectScope": true,
"injectVariableName": true,
},
"warn": Object {
"injectFileName": true,
"injectScope": true,
"injectVariableName": true,
},
}
`;

exports[`buildOptions should return defaults for invalid args 3`] = `
Object {
"debug": Object {
"injectFileName": true,
"injectScope": true,
"injectVariableName": true,
},
"error": Object {
"injectFileName": true,
"injectScope": true,
"injectVariableName": true,
},
"exception": Object {
"injectFileName": true,
"injectScope": true,
"injectVariableName": true,
},
"info": Object {
"injectFileName": true,
"injectScope": true,
"injectVariableName": true,
},
"log": Object {
"injectFileName": true,
"injectScope": true,
"injectVariableName": true,
},
"warn": Object {
"injectFileName": true,
"injectScope": true,
"injectVariableName": true,
},
}
`;

exports[`buildOptions should return defaults for invalid args 4`] = `
Object {
"debug": Object {
"injectFileName": true,
"injectScope": true,
"injectVariableName": true,
},
"error": Object {
"injectFileName": true,
"injectScope": true,
"injectVariableName": true,
},
"exception": Object {
"injectFileName": true,
"injectScope": true,
"injectVariableName": true,
},
"info": Object {
"injectFileName": true,
"injectScope": true,
"injectVariableName": true,
},
"log": Object {
"injectFileName": true,
"injectScope": true,
"injectVariableName": true,
},
"warn": Object {
"injectFileName": true,
"injectScope": true,
"injectVariableName": true,
},
}
`;
Empty file added src/utils/index.js
Empty file.
36 changes: 36 additions & 0 deletions src/utils/pluginOptions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
const defaultSettings = {
injectScope: true,
injectVariableName: true,
injectFileName: true,
};

const defaultMethods = ['debug', 'error', 'exception', 'info', 'log', 'warn'];

// this should deep merge in the furture when we are dealing with more than just flags
const mergeOptions = options => {
const sanitizedOptions = Object.keys(options || {})
.filter(key => Object.keys(defaultSettings).includes(key))
.reduce(
(acc, key) => ({
...acc,
[key]: options[key],
}),
{}
);
return Object.assign({}, defaultSettings, sanitizedOptions);
};

export const buildOptions = userOptions => {
// remove ignore patterns from settings since it has been consumed already
// eslint-disable-next-line no-unused-vars
const { methods, ignorePatterns, ...options } = userOptions || {};

// output spreads the flags over each method
// in the future this could be expanded to allow method level config
return (methods || defaultMethods).reduce((acc, method) => {
return {
...acc,
[method]: mergeOptions(options),
};
}, {});
};
24 changes: 24 additions & 0 deletions src/utils/pluginOptions.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { buildOptions } from './pluginOptions';

describe('buildOptions', () => {
test('should return defaults for invalid args', () => {
let actual = buildOptions();
expect(actual).toMatchSnapshot();
actual = buildOptions(undefined);
expect(actual).toMatchSnapshot();
actual = buildOptions({});
expect(actual).toMatchSnapshot();
actual = buildOptions({ unknownFlag: false });
expect(actual).toMatchSnapshot();
});

test('should allow methods to be changed', () => {
let actual = buildOptions({ methods: ['debug'] });
expect(actual).toMatchSnapshot();
});

test('should overwrite defaults', () => {
let actual = buildOptions({ injectFileName: false });
expect(actual).toMatchSnapshot();
});
});

0 comments on commit 70e987b

Please sign in to comment.