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

Use locally installed react-tools #10

Closed
Munter opened this issue Aug 8, 2014 · 4 comments · Fixed by #11
Closed

Use locally installed react-tools #10

Munter opened this issue Aug 8, 2014 · 4 comments · Fixed by #11

Comments

@Munter
Copy link
Contributor

Munter commented Aug 8, 2014

Depending on a specific version of react-tools, while the user might be using a different one can lead to problems. I've run into similar issues for every precompiler out there.

The best solution is to do a try/catch for requiring react-tools and throw an error if it is not installed, hopefully with a helpful error message on how to fix the problem

@tillarnold
Copy link
Owner

The best solution is to do a try/catch for requiring react-tools and throw an error if it is not installed, hopefully with a helpful error message on how to fix the problem

This is a good solution. I'll do that.

I initially wanted to add react-tools (without a version number) as a peerDependencie. But then it would not work when react-tools is installed globally, or would it?

@Munter
Copy link
Contributor Author

Munter commented Aug 8, 2014

Peerdecpendencies are are real pain to work with. We've been wrapping compilers like this in assetgraph for years, and when we used peer dependencies there was always something broken either in a dev setup with linked dependencies, with globally available modules or even worse, in production.

Just move react-tools down to a dev-dependency so you can still run all tests on this repository when developing

@tillarnold
Copy link
Owner

Ok I'll do that.

@Munter
Copy link
Contributor Author

Munter commented Aug 8, 2014

https://www.npmjs.org/package/accord might be an interesting project for this. Allthough there is no jsx support yet. I think we need it for assetgraph pretty soon, so I'll probably send a pull request to add it to accord.

I'll see if I can remember to trickle it back here as well when it's done

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 a pull request may close this issue.

2 participants