search

dowjones / gulp-bundle-assets

Create static asset (js, css) bundles from a config file: a common interface to combining, minifying, revisioning and more
MIT License
133 stars 36 forks source link

Keep bundle order in bundle.result.json #86

Closed PlasmaPower closed 8 years ago

PlasmaPower commented 8 years ago

Resolves #71

PlasmaPower commented 8 years ago

Build's failing on node <= 0.12, a bit of an odd error but I'm going to temporarily downgrade by node version to test it. I think it might be the use of Object.keys, but IDK.

PlasmaPower commented 8 years ago

That should be fixed - for whatever reason the fs.writeFileAsync implementation wasn't getting a String, just something that looked like a String (but didn't have indexOf). Calling the String constructor on it if it isn't a string seems to have fixed it.

chmontgomery commented 8 years ago

Thanks for the PR! I'm currently out of town but can take a look at this first thing Monday On May 20, 2016 4:05 PM, "PlasmaPower" notifications@github.com wrote:

That should be fixed - for whatever reason the fs.writeFileAsync implementation wasn't getting a String, just something that looked like a String (but didn't have indexOf). Calling the String constructor on it if it isn't a string seems to have fixed it.

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/dowjones/gulp-bundle-assets/pull/86#issuecomment-220718423

PlasmaPower commented 8 years ago

@chmontgomery Sounds good!

PlasmaPower commented 8 years ago

@AndrewCraswell does this work for your use case?

AndrewCraswell commented 8 years ago

@PlasmaPower, so long as this PR gets merged, it looks good! Thanks for doing the work, this will be a huge help to me and my team :).

If you'd like to also update the documentation and the existing example code to describe the enhancement, I'd be happy to Venmo or Square Cash you an extra $15. I think this is the only example you'd probably need to modify: https://github.com/dowjones/gulp-bundle-assets/tree/master/examples/full

PlasmaPower commented 8 years ago

@AndrewCraswell Glad I could help! I'm not sure how to describe the enhancement in any of the examples since none of them depend on the order of the generated JSON if I've looked through them correctly. I have, however, rebuilt the result JSON since that changed with my change. Is there anything else I should add there?

I've also added a note about order in the docs and readme. As for the extra $15 - thanks! If you could add it to the bounty on Bountysource or send it in Bitcoin (1MVL1ygkgRSvhxC5PwbGqdZh26XSYV2sGD) that would be great, but I just setup $PlasmaPower on Square Cash if you'd prefer that.

PlasmaPower commented 8 years ago

@AndrewCraswell It looks like I'd have to enter in my debit card to receive money from square cash (which I'd prefer not to do for security reasons) so the $15 is yours :). Documentation should be included in a PR anyway.

chmontgomery commented 8 years ago

👍 looks good to me. I have one minor update as a followup to this change so that the scripts and styles keys will always stay in order as well.

PlasmaPower commented 8 years ago

@chmontgomery Makes sense - there was a race condition there.