Provide default for defaultLayout

[jfbrennan]

It pains me to see this:

app.engine('handlebars', exphbs({defaultLayout: 'main'}));

This PR makes express-handlebars default to main if defaultLayout is not set. That way you can do:

app.engine('handlebars', exphbs());

[UziTech]

shouldn't the default be 'main'?

[jfbrennan]

We can name it whatever, happy to go with main if that’ll get this PR merged

[jfbrennan]

@ericf any objections to this PR?

[UziTech]

/cc @sahat

[jfbrennan]

@UziTech @ericf @sahat can I get a review? This is a simple backwards-compatible change that completes the "sensible defaults" story for express-handlebars.

[jfbrennan]

Thanks @UziTech when do you expect the next release to happen?

[UziTech]

I don't have the ability to merge pull requests and it looks like @ericf has stepped away from the project. So @sahat is the only active contributor with the ability to merge PRs and release new versions.

[jfbrennan]

@UziTech @sahat now that this suggestion has been approved I updated the docs to reflect the change.

[sahat]

Sorry for the late reply, the change looks good to me, I am going to go ahead and merge it.

[sahat]

You can find the latest express-handlebars v3.1.0 on npm, which includes this change, as well as the updated handlebars v4.1.2 dependency.

[calebolin]

This wasn't backwards-compatible. 😢

[jfbrennan]

@calebolin how so?

Previously you had to exphbs({defaultLayout: 'filename'}) and you can continue to do that with the new version. If you exphbs() that works now as well with defaultLayout defaulting to 'main'

[UziTech]

express-handlebars could be used without a layout when using exphbs() which has to be changed to exphbs({defaultLayout: null}) with this change.

This should bump the major version #253

[calebolin]

@jfbrennan exactly what @UziTech said

[jfbrennan]

No layout at all...never had that use case 🤔

[calebolin]

IMHO it doesn't matter whether a use case can be imagined. If the API supports it, and a release breaks that API, it should be a major version bump.

[smebberson]

Yes, this should have been a major version bump.

[guanzo]

Broke for me too

[donmccurdy]

This PR broke an example application I provide to other developers, and I've received complaints from those developers. Code that previously worked without a layout now throws an error, without intentionally updating this dependency:

Error: ENOENT: no such file or directory, open '<project-path>/views/layouts/main.hbs'

Please consider reverting the change in the 3.x lifecycle and re-releasing it in a 4.x release, although that's yet another API-breaking change so I could see the argument for leaving this alone now that it's done. See https://semver.org/. Thanks!

[twbartel]

I have the same issue that @donmccurdy has, and that is currently causing me a lot of unnecessary work. Therefore, I support his request.

[jfbrennan]

For those who were broken does @UziTech's defaultLayout: null not resolve your issues?

I think the change is a positive one, but could have done my part to help the maintainers identify non-backwards compatible use cases.

[twbartel]

@jfbrennan Yes, it does resolve the issue. Of course, the code to adapt is fairly minimal and takes only 5 minutes. What was quite a bit of work, in my case, was:

• Figuring out what my clients complained about
• Figuring out why that happens, and which npm package to focus on
• Finding this discussion and understanding the issue
• Deciding on a way forward (pin version to 3.0 or go with 3.1 or something else?)
• Making a change to a training video
• Communicating the change

All in all, I estimate I lost 4-5 hours due to this change. I was rather unhappy with this, because it produced no value for anybody.

[kevfromireland]

Just encountered this breaking change today myself.

FWIW, I really appreciate the effort you put into the library and the hassle it has saved me. The value it has provided me over the past year of using it nicely balances out the few hours today I had to burn :)

[PaulBehrendtVentoro]

hit us too, why still no 3.1.1? release this as v4

[kylepeeler]

This should have been a major version bump, breaking change... follow sem-ver...