ericf / express-handlebars

A Handlebars view engine for Express which doesn't suck.
BSD 3-Clause "New" or "Revised" License
2.31k stars 383 forks source link

Define Handlebars as a peer dependency #74

Closed knpwrs closed 10 years ago

knpwrs commented 10 years ago

This change defines handlebars as a peer dependency. What this basically means is that users can define whatever version of handlebars they want to use in their package.json as a dependency and express-handlebars will automatically use that version. If they do not specify handlebars as a dependency then the latest version matching the version in our package.json will be installed automatically.

The really great thing about this is that users can do their own stuff with handlebars in addition to using this module and handlebars is only ever loaded once without having to use npm dedupe. Other modules which define a peer dependency on handlebars can also be installed and they will load the same handlebars code that we do.

I have also defined handlebars as a dev dependency so that going into the future any unit tests from #23 will work.

Current directory structure after running npm i express-handlebars in an empty directory:

.
└── node_modules
    └── express-handlebars
        └── node_modules
            └── handlebars

And after my changes running npm i git://github.com/KenPowers/express-handlebars.git in an empty directory:

.
└── node_modules
    ├── express-handlebars
    └── handlebars
ericf commented 10 years ago

@KenPowers the main feature here—using your own version of Handlebars—has been supported since this first version of this package; it was one of the main goals/reasons for creating this. You can specify your own version like this:

var express    = require('express'),
    exphbs     = require('express-handlebars'),
    Handlebars = require('handlebars');

var app = express();

app.engine('hbs', exphbs({handlebars: Handlebars}));
...

Details here: https://github.com/ericf/express-handlebars#handlebarsrequirehandlebars


Peer Dependencies is not very well supported in npm and leads to issues in more complicated applications. It seems like there might be an issue when using npm's de-duplicaiton feature? Could you elaborate more on that?

knpwrs commented 10 years ago

I understand that custom Handlebars is already supported. This just makes it easier as there's nothing to programmatically configure and more friendly with other modules which may define peer dependencies.

The only issue with npm dedupe is that it has to be run explicitly. Again, this just makes the whole process more streamlined.

As for the version, I just kept what was already there. I'm not familiar with any API changes between 1.x and 2.x but if express-handlebars can support any version of handlebars then the dependency version can be relaxed a bit.

ericf commented 10 years ago

I'm reluctant to rely on npm's peerDependencies feature because there are many open issues with it. We've also been bit by it before, forcing app developers to jump through hoops to get around the issues.

My guess is that using this package's feature of providing a specific version of Handlebars is seldom used. I'm fine with it being explicit in the code that a specific version of Handlebars is being passed directly to this package, instead of it happening because of metadata.

If npm shows interest in supporting peerDependencies and is willing to iron out all the edge cases, then I think we can revisit this. Thanks for taking the time to suggest this change.