-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Babel > 5 's es6 module syntax output breaks rewire-webpack #1337
Comments
I don't think this is a problem with Babel. I'm not going to change the way modules are generated because it's necessary to support live bindings. Let me know if I can be of any further assistance though. |
Seems reasonable. Then is there any way (general Babel API, plugin-level API...) to get the bound name? |
There's no public API but I guess you could do: import foo from "foo"; var Transformer = require("babel-core").Transformer;
module.exports = new Transformer("my-plugin", {
Program: function (node, parent, scope, file) {
file.moduleFormatter.remaps.foo; // AST of _foo.default
}
}); |
I'll try that. Thanks! |
For cross reference, please see: jhnns/rewire#55 (comment) |
The current design breaks usages of |
Babel assumes all input code is module code which means it has an implicit strict mode. This means the arguments for |
That assumption is valid for most of the code out there. But |
Wasn't aware of this but upon checking the spec you're right. Looks like it's a spec inconsistency with Acorn then since Regardless of your strong feelings against this, there's really nothing I can do, sorry. I'm not willing to ditch spec compliant live module bindings just to support the rare case that someone wants to |
I totally understand this, it is really an edge-case. But There is nothing I can do... 😞 |
It seems like it might be easier to have a replacement for rewire that's written to process an AST, then you could easily have a babel plugin to inject a function to read/write all top-level module variables, and if one of those modules happened to be exported, Babel's module export logic would just work. |
Maybe problem with your fork since original Acorn parses it without error. |
Wait, nope, my bad. Can't for the life of me remember the reasoning. |
That is nasty ... but probably just as nasty as using However, that is a lot of work just to support babel :( |
If this issue will most likely never get resolved then I need to begin migrating all of my tests away from Rewire -- off the top of anyone's head, does anyone know of a library similar to Rewire that works with ES6+ syntax? Such a shame! Rewire seemed to work fine with Babel 4. |
At the end of the day, rewire works on CommonJS modules. I'd say that while Babel can create CommonJS modules, it seems like assuming that the CommonJS structure would map directly to ES6 modules is a mistake. That said, there is nothing stopping you from writing your Babel modules as CommonJS and just not using ES6 modules. |
Right, that's how I've gotten around the issue for the time being, but this isn't supportable in the long run. |
+1 @damassi. If the solve is "don't use ES6 imports", I and my co-workers are in the same boat and will unfortunately need to migrate away from Rewire. |
I don't know too much about ES6 module details, but afaict it's not correct that rewire is not compatible with the ES6 module syntax itself. It is not compatible with babel, because babel renames variables based on the assumption that all variables can be statically analyzed – which is only true for ~99.99% of JavaScript. |
@jhnns Except rewire works under a similar assumption where the local variable is named exactly the same as the source. |
Yes, I'm writing code under the assumption that my variables are named exactly the same as in the source :) |
So ... just loud thinking: If rewire would use an AST, then it'd turn this module: let a = 1;
let b = 2; into let a = 1;
let b = 2;
export let __set__ = function (variable, value) {
let setters = {
a(value) {
a = value;
},
b(value) {
b = value;
}
};
setters[variable](value);
};
export let __get__ = function (variable) {
let getter = {
a() {
return a;
},
b() {
return b;
}
};
return getter[variable]();
}; and babel would be fine? |
Yeah, that's what I was getting at. I'd expect that to work. |
And walking the top-level VariableDeclaration nodes in the file would be pretty straightforward. |
I haven't worked with an AST parser yet, but yes... it probably is. This change would break the "feature" that allowed to use dot notation when calling Nonetheless it would be a breaking change... and I still assume it to be a lot of work. It needs to be added to rewire-webpack as well. |
Here's a plugin I whipped up in a few minutes, it does the exact transformation specified in this comment: var Transformer = require("babel-core").Transformer;
var t = require("babel-core").types;
module.exports = new Transformer("rewire", {
Program: function (node, parent, scope, file) {
this.stop(); // optimisation: stops the traversal from traversing the rest of the AST
var bindings = Object.keys(scope.bindings);
var buildBindingObject = function (buildBody) {
var obj = t.objectExpression([]);
for (var i = 0; i < bindings.length; i++) {
var name = bindings[i];
var id = t.identifier(name);
obj.properties.push(t.property(
"init",
id,
t.functionExpression(t.identifier("_" + name), [], t.blockStatement([buildBody(id, name)]))
));
}
return obj;
};
var buildExportDeclaration = function (name, params, buildBody) {
params.unshift(t.identifier("name"));
var block = t.blockStatement([
t.returnStatement(
t.callExpression(
t.memberExpression(buildBindingObject(buildBody), params[0], true),
[]
)
)
]);
var func = t.functionDeclaration(
t.identifier(name),
params,
block
);
return t.exportNamedDeclaration(func);
};
var nodes = this.get("body");
nodes[nodes.length - 1].insertAfter([
buildExportDeclaration("__set__", [t.identifier("value")], function (id) {
return t.expressionStatement(t.assignmentExpression("=", id, t.identifier("value")));
}),
buildExportDeclaration("__get", [], function (id) {
return t.returnStatement(id);
})
]);
}
}); Spaghetti code, especially the |
Thanks @sebmck! |
@sebmck there is a problem in the code:
Not sure how to solve this problem, but rewire plugin must ignore |
@loganfsmyth main idea of rewire is to give ability to override any variable in global scope regardless from type. It's not supposed to be used in production, but in tests it plays significant role. For example I have
Then I will be able to check how my module behaves when Of course I can use In this case |
@kossnocorp have you found a solution to your first point in #1337 (comment) as i am having the same issue. |
@speedskater nope, but you can try to remove all the |
@kossnocorp Does that mean that all I can't find any great alternatives to rewire, and downgrading babel isn't a long term solution, so any guidance here, even if rewire is just "mostly" usable, would be great (outside of using require instead of import). |
@virajsanghvi I didn't checked described solution, but yeah, I beleave if you want to use it globally you have to remove all the Personally I still use rewire and old-fashion |
Rewiring Concerning babel compatibility: I'm thinking about implementing the AST solution as pointed out here, but that requires a major rewrite which I currently don't have the time for 😞 |
Based on the prototype from @sebmck I wrote a small babel plugin which transforms the import statements into commonjs require statements. Furthermore its adds the methods Rewire, GetDependency and ResetDependency to the default export declaration. var Transformer = require("babel-core").Transformer;
var t = require("babel-core").types;
var GettersArray = t.identifier("__$Getters__");
var SettersArray = t.identifier("__$Setters__");
var ReSettersArray = t.identifier("__$Resetters__");
module.exports = new Transformer("rewire", {
Program: function(node) {
var gettersArrayDeclaration = t.variableDeclaration('let', [ t.variableDeclarator(GettersArray, t.arrayExpression([])) ]);
var settersArrayDeclaration = t.variableDeclaration('let', [ t.variableDeclarator(SettersArray, t.arrayExpression([])) ]);
var resettersArrayDeclaration = t.variableDeclaration('let', [ t.variableDeclarator(ReSettersArray, t.arrayExpression([])) ]);
var nameVariable = t.identifier("name");
var valueVariable = t.identifier("value");
var universalGetter = t.exportNamedDeclaration(t.functionDeclaration(
t.identifier('__GetDependency__'),
[nameVariable ],
t.blockStatement([
t.returnStatement(t.callExpression(t.memberExpression(GettersArray, nameVariable, true), []))
])
));
var universalSetter = t.exportNamedDeclaration(t.functionDeclaration(
t.identifier('__Rewire__'),
[nameVariable, valueVariable ],
t.blockStatement([
t.expressionStatement(t.callExpression(t.memberExpression(SettersArray, nameVariable, true), [valueVariable]))
])
));
var universalResetter = t.exportNamedDeclaration(t.functionDeclaration(
t.identifier('__ResetDependency__'),
[ nameVariable ],
t.blockStatement([
t.expressionStatement(t.callExpression(t.memberExpression(ReSettersArray, nameVariable, true), []))
])
));
node.body.unshift(gettersArrayDeclaration, settersArrayDeclaration, resettersArrayDeclaration, universalGetter,
universalSetter, universalResetter);
return node;
},
ImportDeclaration: function(node, parent, scope, file) {
var requireVariableDeclarators = [];
var originalVariableDeclarations = [];
var getters = [];
var setters = [];
var resetters = [];
var accessors = [];
var mappedSpecifiers = node.specifiers.forEach(function(specifier) {
var localVariable = specifier.local;
var localVariableName = localVariable.name;
var getter = t.identifier('__get' + localVariableName + '__');
var setter = t.identifier('__set' + localVariableName + '__');
var resetter = t.identifier('__reset' + localVariableName + '__');
var originalVariableIdentifier = t.identifier('__' + localVariableName + 'Orig__');
var requireExpression = t.callExpression(t.identifier('require'), [node.source]) ;
if(specifier.type !== "ImportDefaultSpecifier" && specifier.type !== "ImportNamespaceSpecifier") {
requireExpression = t.memberExpression(requireExpression, specifier.imported || specifier.local, false);
}
requireVariableDeclarators.push(t.variableDeclarator(
specifier.local,
requireExpression
));
originalVariableDeclarations.push(t.variableDeclaration('const', [ t.variableDeclarator(originalVariableIdentifier, localVariable)]));
function addAccessor(array, operation) {
accessors.push(t.expressionStatement(t.assignmentExpression("=", t.memberExpression(array, t.literal(localVariableName), true), operation)));
}
getters.push(t.functionDeclaration(
getter,
[],
t.blockStatement([
t.returnStatement(localVariable)
])
));
addAccessor(GettersArray, getter);
setters.push(t.functionDeclaration(
setter,
[t.identifier("value")],
t.blockStatement([
t.expressionStatement(t.assignmentExpression("=", localVariable, t.identifier("value")))
])
));
addAccessor(SettersArray, setter);
resetters.push(t.functionDeclaration(
resetter,
[],
t.blockStatement([
t.expressionStatement(t.assignmentExpression("=", localVariable, originalVariableIdentifier))
])
));
addAccessor(ReSettersArray, resetter);
});
return [t.variableDeclaration('let', requireVariableDeclarators)].concat(originalVariableDeclarations).concat(setters).concat(getters).concat(resetters).concat(accessors);/*.
concat(originalVariableDeclarations).concat(setters).concat(resetters);*/
},
ExportDefaultDeclaration: function(node) {
return t.exportDefaultDeclaration(t.callExpression(t.memberExpression(t.identifier('Object'), t.identifier('assign')), [ node.declaration, t.objectExpression([
t.property('init', t.literal('__Rewire__'), t.identifier('__Rewire__')),
t.property('init',t.literal('__ResetDependency__'), t.identifier('__ResetDependency__')),
t.property('init',t.literal('__GetDependency__'), t.identifier('__GetDependency__'))
])]));
}
}); To use it: import MyModule from 'pathToModule.js';
MyModule.__Rewire__('DependentModule', {
theMocked: 'property'
}); Afterwards: MyModule.__ResetDependency__('DependentModule'); @sebmck: Do you see any drawbacks using this techniques for testing purposes? If not I will publish this plugin as an npm package in the following days. |
@speedskater - when you are ready to publish could you please notify this thread? |
@speedskater Unsure about the correctness, you're likely to run into compatibility issues with existing ES6 since you're changing the semantics quite dramatically but I don't know for sure. |
@sebmck You are right. Converting the import into require statements can cause troubles when using other module loaders. Therfore I changed the implementiation regarding the ImportDeclaration. The original import is replaced by another es6 import to a new artficial variable and a temporary variable with the original import name. This change should make the plugin independent of the actual module loading system used. The updated handling of the import declaration. ImportDeclaration: function(node, parent, scope, file) {
var variableDeclarations = [];
var getters = [];
var setters = [];
var resetters = [];
var accessors = [];
node.specifiers.forEach(function(specifier) {
var localVariable = specifier.local;
var localVariableName = localVariable.name;
var getter = t.identifier('__get' + localVariableName + '__');
var setter = t.identifier('__set' + localVariableName + '__');
var resetter = t.identifier('__reset' + localVariableName + '__');
var requireExpression = t.callExpression(t.identifier('require'), [node.source]) ;
var actualImport = scope.generateUidIdentifier(localVariableName + "Temp");
specifier.local = actualImport;
variableDeclarations.push(t.variableDeclaration('let', [ t.variableDeclarator(localVariable, actualImport)]));
function addAccessor(array, operation) {
accessors.push(t.expressionStatement(t.assignmentExpression("=", t.memberExpression(array, t.literal(localVariableName), true), operation)));
}
getters.push(t.functionDeclaration(
getter,
[],
t.blockStatement([
t.returnStatement(localVariable)
])
));
addAccessor(GettersArray, getter);
setters.push(t.functionDeclaration(
setter,
[t.identifier("value")],
t.blockStatement([
t.expressionStatement(t.assignmentExpression("=", localVariable, t.identifier("value")))
])
));
addAccessor(SettersArray, setter);
resetters.push(t.functionDeclaration(
resetter,
[],
t.blockStatement([
t.expressionStatement(t.assignmentExpression("=", localVariable, actualImport))
])
));
addAccessor(ReSettersArray, resetter);
});
return [node].concat(variableDeclarations).concat(setters).concat(getters).concat(resetters).concat(accessors);
} |
Finally published the babel-rewire-plugin. |
If you name it On Tue, May 12, 2015 at 2:24 PM, speedskater notifications@github.com
|
@sebmck Thanks renamed the project and republished it https://github.com/speedskater/babel-plugin-rewire |
I really appreciate this @speedskater; will spread the word. |
@speedskater Nice! |
Thx @speedskater for investigating this. Could you explain in bulletpoints what your code does? As far as I understand, it looks for And why did you opt for another API than rewire? It looks like your plugin is not actually compatible with rewire. |
@jhnns The code generates the methods Rewire, GetDependency and ResetDependency and internally dispatches them to the corresponding getters and setters, which allow to modify the respective imported models. e.g.: import MyModule from 'path/to/MyModule.js';
export default ""; will be transformed to: "use strict";
export { __GetDependency__ };
export { __Rewire__ };
export { __ResetDependency__ };
import _MyModuleTemp from "path/to/MyModule.js";
var __$Getters__ = [];
var __$Setters__ = [];
var __$Resetters__ = [];
function __GetDependency__(name) {
return __$Getters__[name]();
}
function __Rewire__(name, value) {
__$Setters__[name](value);
}
function __ResetDependency__(name) {
__$Resetters__[name]();
}
var MyModule = _MyModuleTemp;
function __setMyModule__(value) {
MyModule = value;
}
function __getMyModule__() {
return MyModule;
}
function __resetMyModule__() {
MyModule = _MyModuleTemp;
}
__$Getters__["MyModule"] = __getMyModule__;
__$Setters__["MyModule"] = __setMyModule__;
__$Resetters__["MyModule"] = __resetMyModule__;
export default Object.assign("", {
"__Rewire__": __Rewire__,
"__set__": __Rewire__,
"__ResetDependency__": __ResetDependency__,
"__GetDependency__": __GetDependency__,
"__get__": __GetDependency__
}); Regarding the compatibility to rewire.js you are right. It was a mistake from my side. I just added the respective methods set and get for compatibility reasons. Regarding the with method, it would imho make more sense to move the respective code into a small library which can work with rewire.js as well as babel-plugin-rewire. In case of BDD style tests I would propose the following API for it (automatically adds afterEach) and could publish it into a new project (e.g. named use-require) the day after tomorrow. @jhns what do you think? import useRewire from 'use-rewire';
import ComponentToTest from 'my-fancy-wrapper-component-module';
ComponentToTest.__Rewire__('ChildComponent', React.createClass({
render: function() { return <div {...this.props}></div>; }
}));
describe('Tests my fancy wrapper module', function() {
var rewire = useRewire();
it('renderTest', function() {
rewire(ComponentToTest, 'ChildComponent', React.createClass({
render: function() { return <div {...this.props}></div>; }
}));
..... test component ....
});
}); |
Ok, so your plugin just adds I'm curious about your solution, because I'm thinking about a similar solution for rewire. But I'd appreciate if you would stick to the rewire API as close as possible because otherwise people will need to rewrite their modules. Then it does not make sense to carry the |
I get your point. And my indent wasn't to deviate too much from rewire.js, although I might have overseen some details ;). Therefore I would propose the following enhancement: As my knowledge of rewire.js is limited and there always will be a functionality I likely will oversee when porting the API into the babel plugin I would suggest to join forces to create a solution which tries to be as compatible as possible to rewire.js while providing module system independent es6 support. |
@speedskater - So i've finally got around to replacing Rewire with your library and its working great. |
I'm trying to use the babel-plugin-rewire as a replacement but without even trying to mock anything I get an error saying |
I think it might be a regression. Please try version 0.1.12 of babel-plugin-rewire until speedskater/babel-plugin-rewire#28 is fixed. |
@0x80 the new version 0.1.14 should fix your issues. |
Please see my comment over their issuetracker -> jhnns/rewire-webpack#12 (comment)
(Actually I'm not sure its Babel > 5 's own problem since I was not using Babel for a bit of time)
The text was updated successfully, but these errors were encountered: