Let bin scripts use the correct npm module for esprima.#54
Let bin scripts use the correct npm module for esprima.#54davidmccabe wants to merge 1 commit intofacebookarchive:fb-harmonyfrom
Conversation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks! |
There was a problem hiding this comment.
Seems fine. Though actually I think upstream should make this require relative (require('../esprima')) and then we wouldn't need to be different. I don't know what the Right Thing™ is though.
There was a problem hiding this comment.
Yea, I think using a relative require makes sense... Want to send such a pull request upstream? Then we can just sync it down.
(Feel free to leave this one open in case they point out a good reason for doing it this way...)
|
I don't know what the right thing is, either. It seems crazy that you should have to change the code just because you changed the npm package name. |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
ghost
added
the
4 participants
The scripts included with esprima in
binwere importing the upstream esprima module, which goes by a different name in npm. This caused anybody using esprima from the command-line to confusingly lack JSX support.