Skip to content
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

Add missing import #5

Closed
wants to merge 1 commit into from
Closed

Add missing import #5

wants to merge 1 commit into from

Conversation

ericsoco
Copy link

@ericsoco ericsoco commented Dec 4, 2015

Minor omission but ended up being pretty hard to track down. For reference, the way this manifested in my project was as an undefined reference to d3 when asserting instanceof d3.selection.

@ericsoco
Copy link
Author

ericsoco commented Dec 4, 2015

My project also lists d3 as a dependency; browserify was therefore bundling d3 for me but it was not accessible to koto. Installing d3 as a global via a CDN script removed the undefined reference, but caused the instanceof d3.selection assertion to fail, presumably because koto was looking at a different install of d3 than the source code using koto (and calling dataBind).

Oddly, this never caused a problem before, but a recent rm -rf node_modules caused things to explode. Not sure how many other users would be impacted similarly but it would be awesome to see a new version out soon!

@mojodna
Copy link

mojodna commented Dec 4, 2015

I suspect that the failing test on Node-5.1 is due to a phantomjs-related peerDependency not being explicitly present in package.json.

@nicksrandall
Copy link
Member

Hey @ericsoco, nice work! Thanks for pointing this out. I have made the changed and fixed the build. It's ironic that my code coverage tool was breaking everything. A new release is out on NPM.

Also, thanks for looking at KotoJS. Although it's still in it's infantsy, I would love any and all feedback on what types of things you plan on using KotoJS for and what ideas you might have on how to improve it's funcationality.

@nicksrandall
Copy link
Member

Same goes to you @mojodna. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants