fix installation from npm3 #12
Conversation
|
Good to know. Thanks! We apologize for not putting any code style info in this repo yet, but plotly.js makes use of 4-space indentation. |
|
Moreover could you add a comment mentioning that new npm@3 behavior at the top of the try block. Thanks again. |
npm3 will flatten node_modules, so the nested node_modules that is used for copying the JSON over from Topojson will not be available in the package's relative node_modules. This commit uses a synchronous check to see if the path is valid and on failure with try to use the path expected from npm3 installations.
90e1c36 to
0e78905
Compare
|
Updated my commit. Let me know if the comment should say something different or be formatted differently. |
|
@justinwoo Looks good. Thanks for doing this. A better fix down the road would completely remove the copy-topojson step. Maybe the individual topojson files should be built in this repo on |
|
👍 Yeah, probably the files being built into the npm published package would be best. |
|
@etpinard perhaps we can make an issue for this and then back link it to this PR?
|
|
also congrats @justinwoo ! First contributor to the open sourced Plotly.js :) 🏆 |
|
🙌 🎉 🍻 |
|
Ahhhhh I just finished this same fix! So close. |
npm3 will flatten node_modules, so the nested node_modules that is used
for copying the JSON over from Topojson will not be available in the
package's relative node_modules. This commit uses a synchronous check to
see if the path is valid and on failure with try to use the path
expected from npm3 installations.
Tested this fix on my repo using a git url:
"plotly.js": git://github.com/justinwoo/plotly.js.git#90e1c36ec12e431"