Skip to content

Commit

Permalink
fix: problems with other plugins' path replacement (#64)
Browse files Browse the repository at this point in the history
* Add a test case for the issue

* Add changes related to failed linting before

* Fix the original issue with path replacement
  • Loading branch information
fatfisz authored and Kent C. Dodds committed May 9, 2018
1 parent e35114c commit 7d8f7a6
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 1 deletion.
26 changes: 26 additions & 0 deletions other/mock-modules/babel-plugin-path-replace/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
const types = require('babel-types')

const problematicVisitor = {
VariableDeclarator: {
enter(path) {
const initPath = path.get('init')

initPath.replaceWith(
types.sequenceExpression([
types.stringLiteral('foobar'),
initPath.node,
]),
)
},
},
}

module.exports = () => ({
visitor: {
Program: {
enter(path) {
path.traverse(problematicVisitor)
},
},
},
})
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
"ast-pretty-print": "^2.0.1",
"babel-core": "7.0.0-beta.3",
"babel-plugin-tester": "^5.0.0",
"babel-types": "^6.26.0",
"babylon": "7.0.0-beta.34",
"cpy": "^6.0.0",
"kcd-scripts": "^0.32.1"
Expand Down
15 changes: 15 additions & 0 deletions src/__tests__/__snapshots__/index.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,21 @@ Error: The macro imported from "./fixtures/non-wrapped.macro" must be wrapped in
`;

exports[`macros when a plugin that replaces paths is used, macros still work properly: when a plugin that replaces paths is used, macros still work properly 1`] = `
import myEval from '../eval.macro'
const result = myEval\`+('4' + '2')\`
global.result = result
↓ ↓ ↓ ↓ ↓ ↓
const result = ("foobar", 42);
global.result = result;
`;

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.,
Expand Down
3 changes: 3 additions & 0 deletions src/__tests__/fixtures/path-replace-issue/.babelrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"plugins": ["path-replace"]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import myEval from '../eval.macro'

const result = myEval`+('4' + '2')`

global.result = result
11 changes: 11 additions & 0 deletions src/__tests__/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -254,5 +254,16 @@ pluginTester({
}
},
},
{
title:
'when a plugin that replaces paths is used, macros still work properly',
fixture: path.join(
__dirname,
'fixtures/path-replace-issue/variable-assignment.js',
),
babelOptions: {
babelrc: true,
},
},
],
})
18 changes: 17 additions & 1 deletion src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,11 @@ function macrosPlugin(babel, {require: _require = require} = {}) {

// eslint-disable-next-line complexity
function applyMacros({path, imports, source, state, babel, interopRequire}) {
const {file: {opts: {filename}}} = state
const {
file: {
opts: {filename},
},
} = state
let hasReferences = false
const referencePathsByImportName = imports.reduce(
(byName, {importedName, localName}) => {
Expand Down Expand Up @@ -153,6 +157,18 @@ function applyMacros({path, imports, source, state, babel, interopRequire}) {
}
const config = getConfig(macro, filename, source)
try {
/**
* Other plugins that run before babel-plugin-macros might use path.replace, where a path is
* put into its own replacement. Apparently babel does not update the scope after such
* an operation. As a remedy, the whole scope is traversed again with an empty "Identifier"
* visitor - this makes the problem go away.
*
* See: https://github.com/kentcdodds/import-all.macro/issues/7
*/
state.file.scope.path.traverse({
Identifier() {},
})

macro({
references: referencePathsByImportName,
state,
Expand Down

0 comments on commit 7d8f7a6

Please sign in to comment.