New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Semver Plugins #691

Open
patcon opened this Issue Oct 29, 2013 · 17 comments

Comments

@patcon

patcon commented Oct 29, 2013

Related: #690

Repurposing the major version of the a plugin's version to specify metadata about a dependency (ie. depends on docpad plugin api v2) does not strike me as the best practice.

http://docpad.org/docs/plugin-write#package-json

The reason reason why we do version 2.0.0 is just a general convention. Version 2 plugins are compatible for with DocPad v6, whereas Version 1 plugins are compatible with DocPad v5 (before DocPad v5 plugins were bundled).

Can we consider using the docpad key in the engine hash of packages.json? Or, if that already has a specific purpose, perhaps we could create a docpad-plugin engine? The current practice would seem to keep a user's plugin from being able to use semver, and likely also prevents them from properly versioning their own project (ie. things like using the 0.x.y convention for pre-stable packages, etc.)

Thanks!

cc: @unframework


Want to back this issue? Place a bounty on it! We accept bounties via Bountysource.

@10xLaCroixDrinker

This comment has been minimized.

Member

10xLaCroixDrinker commented Oct 29, 2013

👍

@bobobo1618

This comment has been minimized.

bobobo1618 commented Oct 29, 2013

Yup. I'm not supporting this with my plugin. package.json has a section for this and will warn the user when they attempt to install an incompatible plugin. I don't see any reason to drop semantic versioning for this. It seems like quite a backwards step.

@RobLoach

This comment has been minimized.

Member

RobLoach commented Oct 30, 2013

Agreed... npm handles the semantic version dependencies for us, so we shouldn't have to worry about this. A plugin depicts what major version of Docpad it uses, not the other way around.

@balupton

This comment has been minimized.

Member

balupton commented Oct 30, 2013

Totally agree.

I thought I posted my thoughts on this somewhere... but can't find the place I did. So I'll just repost here.

The reason this came to be, is that DocPad was around before NPM was around. Then when DocPad Plugins actually became NPM packages, NPM did not yet support peerDependencies which is the new convention for such things. Hence why you would have seen "engines": {"docpad": "6"} a few times, instead of "peerDependencies": {"docpad": "6"}. So we had to address this some other way, as npm couldn't.

Today, this still remains an issue I believe. Let's say if we are DocPad v6 in a day when DocPad v7 is out, and we do npm install docpad-plugin-someplugin that use to have "peerDepndencies": {"docpad": "6"} in it but now has "peerDependencies": {"docpad": "7"} in it, it will install the latest version (the docpad v7 version) and then complain how it is not compatible.

Ideally, this aforementioned issue should be fixed in NPM, by it only installing the latest version is compatible with all peerDependencies. Until that point, the v1 for DocPad v5, and v2 for DocPad v6, v3 for DocPad v7, convention makes the most sense. Hence why docpad install someplugin, actually aliases to npm install --save docpad-plugin-someplugin@2 in DocPad v6, to ensure compatibility in a DocPad v7 world.

As a workaround, and something I'm looking into to address plugin upgrade notifications. Is to have docpad/helper pull in the NPM registry information for the plugins, and use that to fix the NPM peerDependencies install issue. By using docpad/helper to tell us which plugin version is the latest compatible with our peerDependencies.

However, as mentioned, this is something that should be addressed in NPM, and I'm happy waiting for now. Maybe NPM has addressed this since peerDependencies was first introduced, if so, it would be unbeknownst to me as I haven't tested it since.

@unframework

This comment has been minimized.

unframework commented Oct 30, 2013

So why not use engines clause instead of peerDependencies?
On 2013-10-29 9:09 PM, "Benjamin Arthur Lupton" notifications@github.com
wrote:

Totally agree.

I thought I posted my thoughts on this somewhere... but can't find the
place I did. So I'll just repost here.

The reason this came to be, is that DocPad was around before NPM was
around. Then when DocPad Plugins actually became NPM packages, NPM did not
yet support peerDependencies which is the new convention for such things.
Hence why you would have seen "engines": {"docpad": "6"} a few times,
instead of "peerDependencies": {"docpad": "6"}. So we had to address this
some other way, as npm couldn't.

Today, this still remains an issue I believe. Let's say if we are DocPad
v6 in a day when DocPad v7 is out, and we do npm install
docpad-plugin-someplugin that use to have "peerDepndencies": {"docpad":
"6"} in it but now has "peerDependencies": {"docpad": "7"} in it, it will
install the latest version (the docpad v7 version) and then complain how it
is not compatible.

Ideally, this aforementioned issue should be fixed in NPM, by it only
installing the latest version is compatible with all peerDependencies.
Until that point, the v1 for DocPad v5, and v2 for DocPad v6, v3 for DocPad
v7, convention makes the most sense.

As a workaround, and something I'm looking into to address plugin upgrade
notifications. Is to have docpad/helper pull in the NPM registry
information for the plugins, and use that to fix the NPM peerDependencies
install issue. By using docpad/helper to tell us which plugin version is
the latest compatible with our peerDependencies.

However, as mentioned, this is something that should be addressed in NPM,
and I'm happy waiting for now. Maybe NPM has addressed this since
peerDependencies was first introduced, if so, it would be unbeknownst to
me as I haven't tested it since.


Reply to this email directly or view it on GitHubhttps://github.com//issues/691#issuecomment-27357934
.

@balupton

This comment has been minimized.

Member

balupton commented Oct 30, 2013

Engines doesn't work for dependencies, only things like the node and npm version. Hence why peerDependencies was introduced. See https://github.com/isaacs/npm/issues/1400#issuecomment-11829492

@unframework

This comment has been minimized.

unframework commented Oct 30, 2013

But it's not a dependency, since plugins are dependencies of the site
itself. It's just a safety check as far as I can tell.
On 2013-10-29 9:12 PM, "Benjamin Arthur Lupton" notifications@github.com
wrote:

Engines doesn't work for dependencies, only things like the node and npm
version.


Reply to this email directly or view it on GitHubhttps://github.com//issues/691#issuecomment-27358067
.

@balupton

This comment has been minimized.

Member

balupton commented Oct 30, 2013

peerDependencies are used to say, I am compatible with these peers. DocPad is a peer, at least when a site is concerned:

  "dependencies": {
    "docpad": "~6.54.0",
    "docpad-plugin-cachr": "~2.2.0",
    "docpad-plugin-cleanurls": "~2.5.1",
    "docpad-plugin-coffeekup": "~2.2.0",
    "docpad-plugin-coffeescript": "~2.2.2",
    "docpad-plugin-eco": "~2.0.2",
    "docpad-plugin-feedr": "~2.6.0",
    "docpad-plugin-marked": "~2.1.1",
    "docpad-plugin-partials": "~2.8.0",
    "docpad-plugin-services": "~2.4.4",
    "docpad-plugin-stylus": "~2.5.0",
    "docpad-plugin-text": "~2.3.1"
  }

It's also used for Backbone extensions, such as QueryEngine, to say, I am compatible with these Backbone versions. As when it comes to DocPad's use of QueryEngine and Backbone, they are dependency peers.

Using engines doesn't provide any checks here, or any enforcement, docpad should never formally be inside engines - it was a hack that we did before peerDependencies was invented to accomplish the purpose of peerDependencies outside of NPM, as NPM didn't have support. Today we hacklishly support both in DocPad on the safety check level: https://github.com/bevry/docpad/blob/ea63ad5fb9d19cb7200e59e733f0269a12e425ae/src/lib/plugin-loader.coffee#L140-L146

Using peerDependencies now that NPM supports it, will output warnings when your site is using incompatible dependencies with each other. If I am trying to run a plugin which has "peerDependencies": {"docpad": "7"} and my site has "dependencies": {"docpad": "6"}, then NPM will yell at you, as it should.

From my understanding though, peerDependencies only helps with post-install checks, rather than pre-install error prevention (only installing a compatible version of the dependency).

Does that help explain things? Perhaps the idea of running DocPad as a global install is the confusing part here, as that can allude to DocPad being an engine, but DocPad as of recent, will actually fire up the local DocPad install, to ensure that DocPad always abides by the site's dependencies.

@unframework

This comment has been minimized.

unframework commented Oct 30, 2013

I.e. if you are doing an explicit check in plugin-loader anyway then might
as well coopt and read the engines clause there. I can see that docs don't
cover using that clause for custom things but NPM won't barf at least.
On 2013-10-29 9:15 PM, "Nick M" uberfob@gmail.com wrote:

But it's not a dependency, since plugins are dependencies of the site
itself. It's just a safety check as far as I can tell.
On 2013-10-29 9:12 PM, "Benjamin Arthur Lupton" notifications@github.com
wrote:

Engines doesn't work for dependencies, only things like the node and npm
version.


Reply to this email directly or view it on GitHubhttps://github.com//issues/691#issuecomment-27358067
.

@unframework

This comment has been minimized.

unframework commented Oct 30, 2013

So fair enough - peerDependencies seems to fit the bill.
On 2013-10-29 9:22 PM, "Benjamin Arthur Lupton" notifications@github.com
wrote:

peerDependencies are used to say, I am compatible with these peers.
DocPad is a peer, at least when a site is concerned:

"dependencies": {
"docpad": "~6.54.0",
"docpad-plugin-cachr": "~2.2.0",
"docpad-plugin-cleanurls": "~2.5.1",
"docpad-plugin-coffeekup": "~2.2.0",
"docpad-plugin-coffeescript": "~2.2.2",
"docpad-plugin-eco": "~2.0.2",
"docpad-plugin-feedr": "~2.6.0",
"docpad-plugin-marked": "~2.1.1",
"docpad-plugin-partials": "~2.8.0",
"docpad-plugin-services": "~2.4.4",
"docpad-plugin-stylus": "~2.5.0",
"docpad-plugin-text": "~2.3.1"
}

It's also used for Backbone extensions, such as QueryEngine, to say, I am
compatible with these Backbone versions. As when it comes to DocPad's use
of QueryEngine and Backbone, they are dependency peers.

Using engines doesn't provide any checks here, or any enforcement, docpadshould never formally be inside
engines - it was a hack that we did before peerDependencies was invented
to accomplish the purpose of peerDependencies outside of NPM, as NPM
didn't have support.

Using peerDependencies now that NPM supports it, will output warnings
when your site is using incompatible dependencies with each other. If I am
trying to run a plugin which has "peerDependencies": {"docpad": "7"} and
my site has "dependencies": {"docpad": "6"}, then NPM will yell at you,
as it should.

From my understanding though, peerDependencies only helps with
post-install checks, rather than pre-install error prevention (only
installing a compatible version of the dependency).


Reply to this email directly or view it on GitHubhttps://github.com//issues/691#issuecomment-27358451
.

@balupton

This comment has been minimized.

Member

balupton commented Oct 30, 2013

I've made an issue on npm here to describe my proposal: https://github.com/isaacs/npm/issues/4055

@unframework

This comment has been minimized.

unframework commented Nov 1, 2013

Meanwhile I still think that peerDependencies behaviour as of now already replicates the same behaviour that plugin version check ~2 did, so maybe it's already OK to deprecate the v2 check even before isaacs/npm responds to your issue? (Unless I'm missing something about the current peerDependencies behaviour)

@balupton

This comment has been minimized.

Member

balupton commented Nov 1, 2013

peerDependency check only kicks in post-install currently, allowing you to install incompatible versions

the v2 check happens pre-install, allowing you to only install compatible versions

@unframework

This comment has been minimized.

unframework commented Nov 1, 2013

It runs preinstall? Docpad happily installed a v3 plugin for us and just
refused to run it at invocation time with a warning.
On 2013-10-31 10:20 PM, "Benjamin Arthur Lupton" notifications@github.com
wrote:

peerDependency only kicks in post-install currently, allowing you to
install incompatible versions

the v2 check happens pre-install, allowing you to only install compatible
versions


Reply to this email directly or view it on GitHubhttps://github.com//issues/691#issuecomment-27543818
.

@balupton

This comment has been minimized.

Member

balupton commented Nov 1, 2013

docpad install plugin will only install v2 plugins, if no plugin version is specified.

@unframework

This comment has been minimized.

unframework commented Nov 1, 2013

Well somehow plugin v3 got through for us despite that check. Docpad
augmenting normal npm logic seems rife with possible misunderstandings like
these - users see a package.json file and may assume that they can also run
npm install directly, which loses all these extra checks. If the extra
checks are necessary then might as well come up with a dedicated plugin
installer that disallows confusion with normal npm process. Or otherwise
trust npm entirely and avoid extra "magic" :)
On 2013-10-31 10:53 PM, "Benjamin Arthur Lupton" notifications@github.com
wrote:

docpad install plugin will only install v2 plugins, if no plugin version
is specified.


Reply to this email directly or view it on GitHubhttps://github.com//issues/691#issuecomment-27544816
.

nishantjr added a commit to njr-nursery/docpad-plugin-markdownit that referenced this issue Apr 3, 2016

nishantjr added a commit to njr-nursery/docpad-plugin-markdownit that referenced this issue Apr 4, 2016

@balupton balupton added this to the Abstract all the things milestone Jun 18, 2016

@balupton balupton changed the title from Reconsider use of plugins' version as plugin engine spec to Semver Plugins Jun 18, 2016

@balupton

This comment has been minimized.

Member

balupton commented Sep 3, 2018

Next version of DocPad will accomplish this. Magic is at https://github.com/bevry/pluginloader

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment