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

Feature/commonjs support #19

Closed

Conversation

mauriciosoares
Copy link
Contributor

This PR adds support for node envs that uses browserify but does not uses babel as a transformer.

Updated tests, in this case I had to add Mocha and Chai for testing the node part, since karma was not built to test envs that are not browser based (correct me if I'm wrong). Don't know if this is the best approach, but was the simplest that I found.

This closes the issue #15

These configs in package.json enables the dist file to be required in commonjs envs without babel.
This commit adds tests to make sure that the browserify bundle will work in the dist file of clipboard.js

This commit adds the mocha and chai modules, since karma doesn't work well with node only tests.

Also splited tests tasks in package.json and updated .gitignore
@zenorocha
Copy link
Owner

Could you try removing https://github.com/zenorocha/clipboard.js/blob/master/karma.conf.js#L23 from here and pass it in the command line? I guess that would prevent us from adding Mocha and Chai again.

Also rename npm run karma > npm run test-browser, npm run mocha > npm run test-server

@mauriciosoares
Copy link
Contributor Author

what you mean with "prevent from adding mocha and chai again"? You mean to remove from package.json as a dev dependency?

I think we need karma-mocha and karma-chai for karma to work correctly, and we need mocha and chai for the server tests to work independently from karma.

Or did I get something wrong?

@mauriciosoares
Copy link
Contributor Author

Ahhh, sorry man, I get it now, I can use karma to run the tests without using a browser.

I'll work on it :)

@zenorocha
Copy link
Owner

Just started reviewing :)

:octocat: Sent from GH.

zenorocha added a commit that referenced this pull request Sep 30, 2015
@zenorocha
Copy link
Owner

Thank you, pull request merged! See changes here.

:octocat: Sent from GH.

@zenorocha
Copy link
Owner

PR merged! Sorry @mauriciosoares, I was wrong about Karma, it always needs a browser to run.

I renamed the test file to module-systems and added a TODO item there.

Would be awesome to have you leading this interoperability effort :)

zenorocha added a commit that referenced this pull request Sep 30, 2015
@zenorocha zenorocha closed this Sep 30, 2015
@mauriciosoares
Copy link
Contributor Author

Hey @zenorocha, I can't promise you I'll have enough time to do these guys fast... But as soon as I have a few, I'll try to take a look at some issues, and work on them. Thanks for the invite.

adoy4mrx_700wa_0

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