-
Notifications
You must be signed in to change notification settings - Fork 182
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
anonymous functions with babel don't work with _design docs in couchdb. #306
anonymous functions with babel don't work with _design docs in couchdb. #306
Conversation
…to a string with an identifier which breaks couchdb
164c7c6
to
2fb5514
Compare
What do you mean by "which then get cast to a string with an identifier"? |
function map () {}.toString() => 'function map () {}' keeps identifier 'map' when a function has an identifier its not parsed by couchdb. see examples |
Ah, I see now. I think that's a bug in CouchDB (some might even debate that it's also a "bug" in Babel). I think your PR doesn't work when e.g. "function foo" is used as a string in a function. In general a valid fix would involve parsing the JavaScript function to an AST to remove the function identiier properly. A workaround would be to place the function directly to a string. |
I agree Its a bug with couch that's made its way into javascript tooling they haven't considered that function () {} is valid. Its nothing new its part of Javascript. naming an anonymous function is actually good practice as it improves stack traces. I disagree with this being a bug with babel. I've submitted a ticket on couch's jira board. https://issues.apache.org/jira/browse/COUCHDB-2939 to see if they'll fix it there. but until then this replace should be added to allow babel users to use this library. With regards to your example function foo as string, could you share more details on the use case? |
I feel that making an AST for one function is overkill in this example. |
Is your suggestion to try this as a workaround? couchDesignDoc('User', {
email: {
map: `function (doc) {
if(doc.type === 'User') {
emit(doc.email, doc);
}
}`
}
}); |
Yes, that workaround should work for the time being. Making an AST is an overkill, but is the bug-free way to fix the issue.. |
correction ^^ (moved the back ticks down into the value) still a bit crap. no linting in your editor |
bugfree way would be for couch to support named functions. |
I did a little digging. It's all about http://kangax.github.io/nfe/ (Function expressions vs. Function declarations) ... An easy fix is to add parenthesis around things. We can either do that in cradle, or in util.js (CouchDB). Proof: https://gist.github.com/sreuter/ac186eda2a83522a19b0bc9305a3db2b Cc @janl for opinions ;) |
Nice work!! :) |
We (at CouchDB) have talked about this extensively when it became clear that we are using a language quirk of SpiderMonkey rather than regular JS language syntax. Long story short: CouchDB is doing it wrong but won’t be fixing it any time soon. There is a bit of a BC-break component to it for us to “just fix” it, but we’ll be addressing this down the road, some time after 2.0. Until then, it’d be great if you could work around this in cradle :) Thank you! |
Thanks @janl for taking the time to comment on this. So guys in this PR i have added https://github.com/flatiron/cradle/pull/306/files#diff-73b7aef1765d18b91942243d634fe790R200 which fixes this on our side. |
turns out theres another way, which is to call toString() on your map/reduce functions. couchDesignDoc('User', {
email: {
map: function (doc) {
if(doc.type === 'User') {
emit(doc.email, doc);
}
}.toString()
}
}); this will allow any linter to validate the functions syntax as well. |
found that babel gives anonymous functions names which then get cast to a string with an identifier which breaks couchdb.
see gist for examples.
https://gist.github.com/export-mike/322e6c340294113096b6
change adds a simple replace to remove function identifier