browserify / factor-bundle

factor browser-pack bundles into common shared bundles
Other
400 stars 27 forks source link

reconstruct outputs every `addHooks` (to make it compatible with watchify) #71

Closed jcppman closed 9 years ago

jcppman commented 9 years ago

fix #68, #63

dgbeck commented 9 years ago

Thanks @jcppman for this patch. However this guy does not work for the case where outputs is an array of streams. I think we may need something like this to fix the issue

https://github.com/Aben/factor-bundle/commit/18692d69d983470857d6d3c6af7b7bd12266714f

jcppman commented 9 years ago

added two tests which will fail before applying the patch and will success after applying the patch

dgbeck commented 9 years ago

Just opened #75 that incorporates these changes and tests, and also added ability for outputs to be specified as a function for the stream case. Thanks @jcppman.

jcppman commented 9 years ago

@dgbeck

In my opinion https://github.com/Aben/factor-bundle/commit/18692d69d983470857d6d3c6af7b7bd12266714f is more of a workaround to skip the part that cause the issue (write to end stream).

BTW how about three-ways merging instead of copy-and-paste? I mean, I'm not quite care if i got my name on the commit or not but i guess that's the way open-sourced things work right?

dgbeck commented 9 years ago

Sorry u are right. did not mean to side step credit. will do the merge in the future. Thx for vocalizing.

Just want to make sure the stream case is covered as we need it. If there is a better way then let's do it that way

jcppman commented 9 years ago

@dgbeck No worry, I'm glad that it's useful to other ppl not just me!

ghost commented 9 years ago

all sorted in the 2.5.0 release

jcppman commented 9 years ago

Fantastic!