Skip to content
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
Open

Semver Plugins #691

patcon opened this issue Oct 29, 2013 · 17 comments

Comments

@patcon
Copy link

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
Copy link

👍

@bobobo1618
Copy link

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
Copy link
Contributor

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
Copy link
Member

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
Copy link

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
Copy link
Member

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
Copy link

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
Copy link
Member

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
Copy link

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.

@unframework
Copy link

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
Copy link
Member

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

@unframework
Copy link

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
Copy link
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
Copy link

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
Copy link
Member

balupton commented Nov 1, 2013

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

@unframework
Copy link

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 Reconsider use of plugins' version as plugin engine spec Semver Plugins Jun 18, 2016
@balupton
Copy link
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
Projects
None yet
Development

No branches or pull requests

6 participants