outmoded / lout

API documentation generator
Other
276 stars 49 forks source link

Prepare for hapi v6.0 #69

Closed hueniverse closed 10 years ago

hueniverse commented 10 years ago

https://github.com/spumko/hapi/issues/1664

Quick review shows that moving plugin.loader(), requiring handlebars directly, and moving to absolute path to the templates directory will allow it to be backwards compatible change.

Marsup commented 10 years ago

Are you deprecating helpersPath as well ?

I haven't been able to make it work with the advised changes, I'm on 5.1 and a structure like this worked better :

engines: {
  html: {
    module: require('handlebars')
  }
}

Still missing the helpers though, working on it.

hueniverse commented 10 years ago

helpersPath should not change since that is always relative to the basePath or path. What's not working?

Marsup commented 10 years ago

The helpers are unknown to handlebars, I've already checked the path is correct. I've pushed my current setup in hapi6 branch if you want to give it a try. What do you think of the need to put it under module ?

hueniverse commented 10 years ago

You should not need to put it under module. The tests don't.

Can you construct a failing helpers test?

Marsup commented 10 years ago

Well it's worse without module (Invalid view options [...] module is required), which is why I went for it.

The current tests already fail with missing helper as you can see on travis.

hueniverse commented 10 years ago

I suspect it has something to do with you cloning the engine module multiple times. I'll look into it tomorrow.

Marsup commented 10 years ago

I think I've found at least one cause of failure, the require on handlebars goes through Hoek.applyToDefaults which seems to break it, but I still need that module key even if I do the require directly.

hueniverse commented 10 years ago

I forgot you are trying to be backwards compatible. Then yes, you need module in the views config.

Marsup commented 10 years ago

Unless there's a compelling reason not to, I'll try to be, it's not a huge effort at least for that 6.0, I might revise that later if things change in hapi.

Marsup commented 10 years ago

Is it normal that hapi gobbles the module part after providing it to plugin.views() ? Right after my original object becomes { html: { module: null } }.

Marsup commented 10 years ago

Well, everything works now. Any thoughts on the changes ? https://github.com/spumko/lout/compare/v4.2.0...hapi6

hueniverse commented 10 years ago

The entire way the options are setup mixing views and non-views settings is very messy. Why is the engine configurable when the paths to the templates are not? I don't understand what should and should not be customizable. Why isn't just the CSS customizable?

hueniverse commented 10 years ago

See hapi6-eran branch.

Marsup commented 10 years ago

I never bothered to change these options since it seemed fined when I took over the project, and people might want to override everything, from the rendering engine to css, so I've kept it that way.

This doesn't work with hapi 5 but I might as well go for hapi 6. I don't know if people follow hapi versions closely.

hueniverse commented 10 years ago

What doesn't work? The tests are hapi-version-specific but the lib code should work on 6 and 5.