Make it watch - Githubissues

HenrikJoreteg / moonboots

A set of conventions and tools for bundle and serving clientside apps with node.js

159 stars 20 forks source link

Make it watch #63

Closed latentflip closed 9 years ago

latentflip commented 9 years ago

Depends on #62 and probably needs tests or something.

wraithgar commented 9 years ago

Watchify definitely needs a default, and we may want to force it to false in non dev mode.

latentflip commented 9 years ago

Totally agree. It should never be on in prod.

Philip Roberts

On 19 Dec 2014, at 00:24, Michael Garvin notifications@github.com wrote:

Watchify definitely needs a default, and we may want to force it to false in non dev mode.

— Reply to this email directly or view it on GitHub.

faiq commented 9 years ago

@latentflip im working on solving a similar problem myself. I want to keep a cache of the bundle in development mode, so i dont have to wait forever for my page to reload. However, i was worried about the race condition that happens when you have a cached bundle and something is updated in the js/ folder, but before the bundle can be rebuilt and saved as the bundled cache the client requests the bundle. Does this mean the client will get an old version of the bundle?

wraithgar commented 9 years ago

@faiq we solve a similar problem in moonboots-hapi with a little function @latentflip wrote https://github.com/wraithgar/moonboots_hapi/blob/master/lib/lock-async-function.js

faiq commented 9 years ago

@wraithgar can you please point me to where you used it! I'm really confused :disappointed: on how I should be using it.

wraithgar commented 9 years ago

https://github.com/wraithgar/moonboots_hapi/blob/master/index.js#L74 then in the actual handler https://github.com/wraithgar/moonboots_hapi/blob/master/index.js#L80-L85

wraithgar commented 9 years ago

Now that I look at your comment again it seems that doesn't solve your specific problem maybe

lukekarrys commented 9 years ago

@latentflip This is great! And I really want it to land to save me from long build times in developmentMode. So I added a commit to address the following:

watchify has a default of false and always gets set to false if developmentMode is false
added watchify:true to the dev test and and added a test that requests the jsSource twice to make sure 1) it doesnt break and 2) it gets coverage up to 100% because then it can set bundle to self.watchedBundle.

Anything else that needs to get discussed for this? Should we go ahead and update to v8 of browserify now that its out?

faiq commented 9 years ago

@latentflip, how did you solve the concerns I posted above, I would really love some guidance. :D

lukekarrys commented 9 years ago

I pulled this branch into my current project using moonboots_hapi and encountered an error when using it with the modulesDir option (which uses browserify.require and the expose option).

It looks like the result of this bug with watchify: https://github.com/substack/watchify/issues/72.

I'm gonna come up with a failing test and then see if one of the workarounds in that issue will fix it.

lukekarrys commented 9 years ago

I made a failing test where if you get jsSource() twice for a watchified bundle, the bundles are different and any file exposed as part of modulesDir wont be able to be required properly.

I tried updating browserify and watchify to see if any of the workarounds in the issue above worked, but none of them did.

Seems like the only way watchify could be included is if we detect both modulesDir and watchify in the config, we throw an error. Otherwise it'll lead to hard to track down bugs where users just get {} as the result of requiring a module. But that seems like not a great idea.

Keeping an eye on the related watchify issues: