HenrikJoreteg / moonboots

A set of conventions and tools for bundle and serving clientside apps with node.js
159 stars 20 forks source link

V2 #24

Closed wraithgar closed 10 years ago

wraithgar commented 10 years ago

Submitting for discussion, not ready to merge yet obviously

wraithgar commented 10 years ago

Ok we're feature complete, please pick this all apart now.

HenrikJoreteg commented 10 years ago

npm test should run all the tests

lukekarrys commented 10 years ago

One thing I liked previously (probably just because I added it, please feel free to interject if no one else uses it) is the ability for the browserify error stack trace to be set as the result of the jsSource so that it will document.write it to the browser if you refresh (in developmentMode only).

Anyone else feel the same way? Or should we scrap it? @latentflip might also have thoughts about this.

wraithgar commented 10 years ago

@lukekarrys the jsSource and cssSource are error-first callbacks so the frameworks should take that error and act accordingly?

latentflip commented 10 years ago

Yeah, maybe this is moonboots_{hapi,express} responsibility, but this'd be awesome. Moonboots hapi drives me kinda crazy when hapi's overly annoying error logging is trying to tell me about a browserify error.

On 7 April 2014 19:23, Michael Garvin notifications@github.com wrote:

@lukekarrys https://github.com/lukekarrys the jsSource and cssSource are error-first callbacks so the frameworks should take that error and act accordingly?

Reply to this email directly or view it on GitHubhttps://github.com/HenrikJoreteg/moonboots/pull/24#issuecomment-39765305 .

Philip Roberts latentflip.com

wraithgar commented 10 years ago

good catch @HenrikJoreteg forgot to add that to package.json, done

lukekarrys commented 10 years ago

@wraithgar @latentflip I'm fine with leaving it up to the framework. Just wanted to bring it up if there was a case for it to be in core, but looks like there is not.

wraithgar commented 10 years ago

@lukekarrys @latentflip this totally sounds like something moonboots-hapi needs I'll add a ticket.

wraithgar commented 10 years ago

ok the more I look at moonboots-hapi and moonboots v2 the less certain I am about this is ready for primetime. Either it works as luke suggests, which means we need to catch errors in production in moonboots-hapi and don't display the error to the browser, or we're not displaying the errors correctly right now. Gonna have to test this.

HenrikJoreteg commented 10 years ago

I'd say we shouldn't publish this as "latest" until we have it working with moonboots_hapi the way we'd like it to.

HenrikJoreteg commented 10 years ago

could certainly put it on npm then tag it as beta, and tag current latest as still being latest.

HenrikJoreteg commented 10 years ago

also, +1 to luke's comment about writing it to the browser, but I think that should live in moonboots_hapi/moonboots_express as well.

wraithgar commented 10 years ago

In development mode, build errors show up in the browser now, no middleware intervention needed (middleware will want to log 'log' events)

wraithgar commented 10 years ago

moonboots2 branch of moonboots-hapi works w/ this branch now just fine and all tests pass.

wraithgar commented 10 years ago

Added timingMode, which is a flag you can use to turn on 'timing' log events from moonboots to do the kinds of performance analysis that started this whole v2 mess.

lukekarrys commented 10 years ago

Should we remove/alter server.js since it doesn't work anymore?

wraithgar commented 10 years ago

Ah yes cleanup. Thanks @lukekarrys. That belongs in moonboots-express which is yet to be written. Then we can link to it in the readme?

lukekarrys commented 10 years ago

Yeah, the "How to use it" section of the readme will also need cleanup. Should that section just be links to the framework libs + the configurable options?