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

Add AMD (Require.js) + CommonJS (browserify) wrapper #104

Closed
wants to merge 3 commits into from

Conversation

thibaudcolas
Copy link

Hi,

here's a PR to add support for two module loaders: Require.js (AMD modules, define) and browserify (Node-ish/CommonJS, module.exports). It should help with the following issues: #63 and #73.

The UMD wrapper is taken from https://github.com/eduardolundgren/gulp-umd (see also https://github.com/umdjs/umd). I changed it a little bit to accomodate the following browserify issue (use window || this for root instead of this).

The AMD / Require.js scenario is fully working, and I added a test case for it (http://127.0.0.1:8000/tests/?amd=1).

The CommonJS / browserify story is a little different. It's also fully working, but there are caveats:

  • To take full advantage of CommonJS compatibility InstantClick would need to be available in npm: it means adding a package.json, and if possible publishing it on the npm repository. After that, users will be able to require('instantclick') instead of require('../../vendor/instantclick-x.x.x/instantclick').
  • Modules are statically bundled by browserify prior to being served on a page: a test case for browserify would need a build system in place, which is out of scope for this PR. On the flip side, I did tests on my computer and uploaded the resulting browserify bundle as a gist. It works very well.

Let me know if it suits you!

@dieulot
Copy link
Owner

dieulot commented Dec 20, 2014

Thanks for this. Currently I’m focused on getting InstantClick out of its hiatus (= shipping InstantClick 3.1), I’ll take a deeper look at this once the project is moving again.

@dieulot
Copy link
Owner

dieulot commented Dec 26, 2014

So I’ve looked a bit at Require and Browserify and I see more disadvantages than advantages in including them, so I don’t plan to merge, sorry. More details in #63 and #73.

@dieulot dieulot closed this Dec 26, 2014
@thibaudcolas
Copy link
Author

Hey there,

no worries, it's already cool that you took some time to look at it. I answered in #63 and I'll stop here.

Take care.

@egoist
Copy link

egoist commented Oct 30, 2017

Just FYI, I built a UMD version of this library two years ago for you to use in a modern bundler:

https://github.com/egoist/instantclick

/related: #156 #63 #73

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