ForbesLindesay / browserify-middleware

express middleware for browserify, done right
http://browserify.org
MIT License
381 stars 66 forks source link

Use watchify for caching #83

Closed ForbesLindesay closed 9 years ago

ForbesLindesay commented 9 years ago

Maintaining our own dynamic caching system separately from @substack has been a constant source of errors. Switching to watchify has the downside that we will be polling for changes constantly, rather than just checking for changes when the user hits refresh. Polling every 100ms should be fine on most people's systems though. The code looks a little more complex than it might seem like it needs to be because I made watchify notify browserify-middleware immediately of any change so that we can ensure that any requests that come in during the 600ms wait period (that's required to ensure the disk has finished working) are queued until the new bundle is ready.

This also makes caching much simpler by pushing the cached object into being part of the browserify handler rather than storing a global mapping between some ID and some cache object.

@adius could you test this out and let me know if it works/if it needs any improvements? @killroy42 this will implement the async postcompile hook that you requested, via a Promise based interface. @gabegorelick, @allain & @scottbrady I'd appreciate if one of you could review this change for me and check it all looks sensible to you. If you are happy with it, feel free to merge and release.

ForbesLindesay commented 9 years ago

I'll merge this today if I don't get any feedback. Comment now if you want to review this before it's published.

adius commented 9 years ago

I'll check it out right now and give feedback

gabegorelick commented 9 years ago

No objection from me. I've been thinking for a while that leveraging watchify makes sense.

On Friday, June 12, 2015, Adrian Sieber notifications@github.com wrote:

I'll check it out right now and give feedback

— Reply to this email directly or view it on GitHub https://github.com/ForbesLindesay/browserify-middleware/pull/83#issuecomment-111452868 .

adius commented 9 years ago

I didn't have any problems … and it is a lot faster. =) Thanks!

scottbrady commented 9 years ago

I tested this code on a fairly complex project (~2MB bundle when minified) that uses transforms (Coffeescript, Jade) and everything is working for me. The new code is definitely much faster.

adius commented 9 years ago

So let's merge it?! @ForbesLindesay

killroy42 commented 9 years ago

Which release will this hit? 10.2.4?