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

Added support for Browserify #97

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

Added support for Browserify #97

wants to merge 4 commits into from

Conversation

vbwx
Copy link
Contributor

@vbwx vbwx commented Jun 22, 2015

Hello,
after discovering Harp I've really wanted to have a way of concatenating JS files without adding a compilation step, so I went ahead and added Browserify to the pipeline. (Only now I've discovered PR #33, but I still want to share.)
It works by detecting references to require/module/exports in both JavaScript and CoffeeScript files, unless they're being declared (We wouldn't want Browserify to interfere with scripts that use their own require function.)
This mechanism works out-of-the-box with .coffee files, however, for raw JS you need to use the filename extension .es (as in ECMAScript), otherwise the file never goes through terraform (from what I've seen).
The languages of browserified files can be mixed, as demonstrated in the test suite.

@kennethormandy
Copy link
Collaborator

First, thanks so much for taking this on! It’s something we’ve wanted in Harp for a while, and to see it put together with tests and everything—nice job. It seems to work really well, I am impressed.

One minor problem, it seems to work great if you’re running harp server in the root of the project, but not if you’re passing in a different directory, ex:

harp server path/to/my-project

If you have a bundle.es file, you’ll get something like:

Cannot find module '/Users/Kenneth/Sites/sintaxi/harp/bundle.es' from '/Users/Kenneth/Sites/sintaxi/harp'

It looks like it’s still going through the minifier, but looking for the file in reference to where harp server was run from, not where the project is. I imagine that’s not a big fix. (Edit: Looks like this also impacts framework style applications, with a public/ directory and a harp.json.)

I also think it might make sense to just do this to all .js files by default, rather than using the .es extension, especially since we have the PR to add Babel support with the .es extension already #82. Not totally sure about that yet though.

We’d also need to add the ability to ignore directories, since you don’t want your node_modules directory in the comiled www directory—we have a PRs for that, I think one was pretty close to what we were looking for. Even without that though, harp compile seems to work fine, it just has the extra node_modules directory as well.

Seriously nice job!

@vbwx
Copy link
Contributor Author

vbwx commented Jun 23, 2015

Thanks, I'm glad I can contribute to the project.
Of course, enabling this feature for .js files is the way to go, but I couldn't get it to work. Very cool that it didn't require a lot of changes.
Of the problems you've pointed out, are any left that I should try to fix?

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

Hey @vbwx, thank you so much for your help on this. We have it merged and tagged in #122 with all your commit history in tact. We’ll be writing more about the change when we get the next release of Harp out with this Terraform update. We’re really excited about Browserify support, thanks for your work on it!

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