HenrikJoreteg / moonboots

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

htmlSource/template function argument #29

Closed lukekarrys closed 10 years ago

lukekarrys commented 10 years ago

After writing another moonboots plugin (this time for just writing files to dir to be served statically) I found myself implementing a template fn as an option again.

I'm starting to think that this belongs in moonboots core. I know @wraithgar has an open issue for something similar in moonboots-hapi https://github.com/wraithgar/moonboots-hapi/issues/23.

I'm thinking something like:

var moonboots = new Moonboots({
  main: 'app.js',
  htmlSource: function (ctx) {
    // Available ctx options:
    // ctx.jsFileName, ctx.cssFileName, ctx.resourcePrefix
    return someHTMLString;
  }
});

And then if the option exists it would just set results.html.source instead of how we build it here https://github.com/HenrikJoreteg/moonboots/blob/master/index.js#L176-L180

Any objections?

wraithgar commented 10 years ago

Template function as an option belongs in the middleware which should then couple with htmlContext. that's exactly what htmlContext is for. So in hapi you could simply do this:

reply.view('myCoolSite', plugin.htmlContext)

As it stands now you'd have to set that route up manually via the exposed clientApp method, but adding a template would be a great next step to override the behavior of the main app route handler.

Closing, as it belongs in moonboots-hapi/moonboots-express but feel free to push back if I'm wrong!

wraithgar commented 10 years ago

I can see how if the template option was in the middleware you'd miss out on being able to write a templated main html to file, but writing the main html to file isn't really where any of the performance increase comes from.

I'm still torn. I think ultimately we may want both? Does that fragment the feature unnecessarily? Is there some way about how express does templating that means that this wraithgar/moonboots-hapi#23 is less than ideal?

lukekarrys commented 10 years ago

After experimenting with render options for moonboots-express I agree with your original comment. I don't think an htmlSource function as I originally envisioned would work cross-plugin.

In moonboots-express I opted for a render function as an option which just gets called with (req, res) so things can be done any of the various express ways (with the values from htmlContext() set on res.locals).

I was being short-sighted after working on moonboots-static, but I think that plugin is an outlier in that it doesn't deal with any sort of server framework.

Closing since I think doing this in each plugin (exactly like https://github.com/wraithgar/moonboots-hapi/issues/23) is the ideal way to fix this from now on.