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

Delete accidentally imported node_modules #2

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kylev
Copy link
Contributor

@kylev kylev commented Jan 24, 2014

This resolves Issue #1 by deleteing node_modules (rather than just .gitignoring its presence) and adding the
dependency on handlebars, without which this does not work.

@kylev
Copy link
Contributor Author

kylev commented Jan 24, 2014

I had to explicitly add the handlebars dep to get the mocha suite to run cleanly.

I thought we would be able to get handlebars via emblem requiring it, but that doesn't seem to be the case.

@kylev
Copy link
Contributor Author

kylev commented Jan 24, 2014

I verified that you can't depend on a package's dependencies in your own package. It doesn't matter that emblem pulls in handlebars. To require('handlebars') we need to declare it as a dependency. This package was sort of "accidentally" working by virtue of having handlebars sitting in the checked-in node_modules directory. To remove that directory, we need to fix the missing dependency in package.json, per my second commit in this request.

Proof via micro-repo: https://github.com/kylev/jstest-deps

@saponifi3d
Copy link

👍 ... package.json is good.

Kyle VanderBeek and others added 2 commits January 24, 2014 10:48
…ng it through emblem.

For some reason, lacking this the mocha test suite will not be able to find handlebars
and bail on the require statement in index.js.
@kylev
Copy link
Contributor Author

kylev commented Jan 24, 2014

Rebased to current master branch, should apply cleanly.

@kylev
Copy link
Contributor Author

kylev commented Feb 8, 2014

Ping? Having these modules meddling with our npm install is a touch confusing...

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.

2 participants