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

browserify coffeescripts #33

Closed
wants to merge 3 commits into from
Closed

browserify coffeescripts #33

wants to merge 3 commits into from

Conversation

zeke
Copy link
Contributor

@zeke zeke commented Jan 26, 2014

This branch adds browserify support to terraform's coffeescript compilation step. I've added tests and created a sample app to demo the behavior: https://github.com/zeke/harp-browserify-test

It would be ideal to also browserify regular old .js files too, but it wasn't immediately clear to me how to do that. Happy to make amendments to this PR with some guidance. :)

cc @sintaxi @michael-lawrence @kennethormandy @kevinsimper, because you all participated on sintaxi/harp#55

PS I didn't revamp the package.json formatting on purpose. I think npm is doing that automatically.

@sintaxi
Copy link
Owner

sintaxi commented Jan 30, 2014

Very cool. Thanks for the PR and the sample app. This is sweet.

Obviously we would want the JavaScript behaviour to match the CoffeeScript behaviour but I think this gets the ball rolling. I'll take a closer look at this shortly. Definitely interested in adding Browserify support.

@kennethormandy
Copy link
Collaborator

Hey @zeke, we totally dropped the ball on this. I tried it out and it seems great. I haven’t been able to test if you can actually use harp compile with this method yet, though. To solve that, I think we’ll need to treat node_modules/ as if it were _node_modules Harp doesn’t try and compile everything inside.

It would also be great if node_modules was recognised in App or Framework style, so could have this structure:

▸ node_modules/
▾ public/
  ▪ index.jade
  ▪ app.coffee
▪ harp.json
▪ package.json
▪ README.md

Or this one:

▸ node_modules/
▪ index.jade
▪ app.coffee
▪ _harp.json
▪ package.json

…and either would work. Plus we will need essentially have a JavaScript preprocessor that just maybe doesn’t do anything except pass this onto Browserify in the same way as you’ve done.

Obviously not requesting someone just takes this work all on, but hopefully this re-starts the conversation. I think these are the only remaining issues to having full support for it. Thanks for your hard work on this, Zeke! I’m definitely going to be using this fork for some things.

@zeke
Copy link
Contributor Author

zeke commented Apr 18, 2014

@kennethormandy thanks for digging into it.

a JavaScript preprocessor that just maybe doesn’t do anything except pass this onto Browserify

This is what I was thinking too, but wasn't sure how best to implement that. If you want to lay the groundwork for that I'd be happy to help test it or whatever..

@kennethormandy kennethormandy modified the milestone: v1.0.0 Jan 6, 2016
@kennethormandy kennethormandy mentioned this pull request Jan 7, 2016
6 tasks
@zeke
Copy link
Contributor Author

zeke commented Apr 1, 2016

Doing a little spring cleaning on my old pull requests. It looks like #97 will hopefully land soon, so I'm gonna close this out.

@zeke zeke closed this Apr 1, 2016
@zeke zeke deleted the browserify branch April 1, 2016 04:55
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.

3 participants