paperjs / paper.js

The Swiss Army Knife of Vector Graphics Scripting – Scriptographer ported to JavaScript and the browser, using HTML5 Canvas. Created by @lehni & @puckey
http://paperjs.org
Other
14.5k stars 1.23k forks source link

Can the node-canvas dependency be made a peer dependency? #1252

Closed kumarharsh closed 7 years ago

kumarharsh commented 7 years ago

On one of my projects using paperjs, every single npm install node tries to install the canvas dependency. This fails (because I don't have cairo, etc) and I since I'm going to use this purely in browser, I will never need node-canvas to be installed. But the way npm install works, since it doesn't find canvas already in the node_modules folder, it tries to install canvas on each try, thus slowing down every single instance of npm install. I believe moving canvas to a peer dependency would solve this issue. Also, it will mean that the developer has to explicitly specify that their project depends on node-canvas, which I find to be more desirable. What do you think?

lehni commented 7 years ago

What's a peer dependency? I never heard of the concept. I agree that with more and more people using NPM for browser libraries, we have to find a better solution.

lehni commented 7 years ago

Related: https://github.com/paperjs/paper.js/issues/1171#issuecomment-269863344

kumarharsh commented 7 years ago

I think this article may explain it better: https://nodejs.org/en/blog/npm/peer-dependencies/

lehni commented 7 years ago

Hmm but this doesn't sound like it would solve the issue. Peer dependencies will have to be installed, no?

kumarharsh commented 7 years ago

Yes, after giving it a little thought I see that it might not be the best way. Let me try figuring out something which solves this issue. I'll ping back with some results soon.

lehni commented 7 years ago

We already define it as an optional dependency, but these get installed by default, unless you choose to exclude it:

npm install paper --no-optional
lehni commented 7 years ago

Here's another idea:

We could have a pure paper module with no dependencies, a paper-headless that only requires paper and jsdom, and a paper-canvas that requires paper, canvas and jsdom. This would allow the maintaining of different, clearly separated targets. It's a shame that NPM does not allow the definition of such targets, with separate dependencies... I could include the process of keeping the versions up to date and in sync as part of the publishing gulp task...

kumarharsh commented 7 years ago

I think there are plugins which might help with this such as Lerna: https://github.com/lerna/lerna

lehni commented 7 years ago

@kumarharsh thanks for the hint, that looks interesting. I shall investigate this further.

lehni commented 7 years ago

I have started implementing this now in 29de03dc3023d730270f0b5b989a2137b40bef0b:

Planend are the modules paper-jsdom and paper-jsdom-canvas, as shim modules that require and handle the dependencies as peer dependencies. It works well in my local tests already.

The core paper module will then have no dependencies itself, but is attempting to load these libraries and fail silently. If the library is loaded through one of the sims, then an exception is thrown if the dependency is missing.

simonemazzoni commented 7 years ago

@lehni any news about this? I see it hasn't been merged into master yet.

lehni commented 7 years ago

I haven't found time yet to work on this...

simonemazzoni commented 7 years ago

@lehni ok, I understand. Could you provide an estimate time? Also, I don't know exactly what you have in mind to "split" the two parts, but if I can help in some way, let me know.

kumarharsh commented 7 years ago

+1. @lehni I'd also like to lend my hand, if you can tell me what to do.

lehni commented 7 years ago

@tom10271 has pointed out a way to achieve this now already, here:

https://github.com/paperjs/paper.js/issues/1308#issuecomment-293213033

Unfortunately I can't provide reliable estimates. I take care of paper.js in my spare time, and am not in a position to predict how much time I can devote to it at the moment...

@kumarharsh the plan is outlined above, but what I haven't figured out is how to handle versions... Will paper-jsdom and paper-jsdom-canvas be version locked to paper? If so, then we need an automated script that produces these and publishes new versions. That's the main missing part right now.

But maybe, the trick found by @tom10271 is good enough to not have to take care of such separate packages?

simonemazzoni commented 7 years ago

@lehni Thanks for the suggestion. That workaround is the one I am using now to workaround the problem.

Regarding the split I think you're right, the version should be locked to paper. My question is, how is generated the paper package for bower? I noticed that it doesn't install the canvas module, but copies only the paper-core, paper-full files.

lehni commented 7 years ago

Bower is for web projects only, the node-canvas module isn't even in its registry : )

simonemazzoni commented 7 years ago

@lehni yes I know. Sorry, bad choice of words. I just self answered my real question. :)

Btw, #1308 (comment) doesn't seem to work. I'm investigating to check if I made something wrong in my package.json definition.

kumarharsh commented 7 years ago

@lehni I don't think the format @tom10271 suggests in the linked comment is recognized by npm. Ref: https://docs.npmjs.com/files/package.json#dependencies Also, when running yarn install, an error is thrown, which points to the same thing:

image

kumarharsh commented 7 years ago

@lehni can you push the changes you've done (as described here) into a new branch, maybe I can take a shot at completing it?

tom10271 commented 7 years ago

I have an updated solution. Please check: https://github.com/paperjs/paper.js/issues/1308#issuecomment-293514789

Thanks

simonemazzoni commented 7 years ago

Tested right now! It works like a charm. Good catch @tom10271

kumarharsh commented 7 years ago

It works. For yarn, you'd need to run yarn install --ignore-scripts. But these are stop-gap solutions, and won't really work in the long run. For example, what if you need another npm package which has optional deps you need? I could go on with these scenarios (have run into a few myself), but I think you get the idea. If I'm lucky, I'll take a shot at untangling paperjs this weekend. Lets see how it goes :)

lehni commented 7 years ago

Yeah we need a proper fix. @kumarharsh I had it working locally, the missing bit is only the automatic publishing of these shim modules with each publishing of paper itself... This needs a gulp task.

lehni commented 7 years ago

e.g.:

paper-jsdom/index.js:

// Just pass through. The rest is handled by the paper.js library itself:
module.exports = require('paper');

paper-jsdom/package.json:

{
  "name": "paper-jsdom",
  "version": "0.10.4",
  "description": "The Swiss Army Knife of Vector Graphics Scripting, packaged for headless use in Node.js",
  "license": "MIT",
  "homepage": "http://paperjs.org",
  "repository": {
    "type": "git",
    "url": "git://github.com/paperjs/paper-jsdom"
  },
  "bugs": "https://github.com/paperjs/paper.js/issues",
  "author": "Jürg Lehni <juerg@scratchdisk.com> (http://scratchdisk.com)",
  "main": "index.js",
  "files": [
    "index.js",
    "README.md"
  ],
  "engines": {
    "node": ">=4.0.0"
  },
  "dependencies": {
    "paper": "0.10.4",
  },
  "peerDependencies": {
    "jsdom": "^9.4.0",
    "source-map-support": "^0.4.0"
  },
  "browser": {
    "jsdom": false,
    "jsdom/lib/jsdom/living/generated/utils": false,
    "source-map-support": false
  },
  "keywords": [
    "vector",
    "graphic",
    "graphics",
    "2d",
    "geometry",
    "bezier",
    "curve",
    "curves",
    "path",
    "paths",
    "canvas",
    "svg",
    "paper",
    "paper.js",
    "paperjs"
  ]
}
lehni commented 7 years ago

The part I wasn't so sure about was "peerDependencies" vs "dependencies".

kumarharsh commented 7 years ago

IMO, you can put jsdom in the dependencies list for paper-jsdom, as the intent of the user is clear that they want jsdom to be installed.

lehni commented 7 years ago

Yeah I agree...

lehni commented 7 years ago

I am on this now, expect v0.10.4 shortly with these new modules.

lehni commented 7 years ago

Alright, it's all published now!

https://www.npmjs.com/package/paper https://www.npmjs.com/package/paper-jsdom https://www.npmjs.com/package/paper-jsdom-canvas

Please test and let me know if it works.

lehni commented 7 years ago

Oh darn, this should really be v0.11.0, not v0.10.4, because it will break existing Node.js projects that require paper with canvas support. I unpublished v0.10.4 and will publish v0.11.0 in a bit.

lehni commented 7 years ago

Alright, v0.11.0 is out now, and the automatic updating of the sub-modules (NPM shim packages) has also been tested now: https://github.com/paperjs/paper.js/blob/develop/gulp/tasks/publish.js#L57

simonemazzoni commented 7 years ago

Tested it, and it looks good to me.

kumarharsh commented 7 years ago

Perfect! 🎉

lehni commented 7 years ago

Great to hear! I was thinking that node folks could also do this instead, in the main project:

  "dependencies": {
    "canvas": "^1.3.5",
    "jsdom": "^9.4.0",
    "paper": "^0.11.2",
    "source-map-support": "^0.4.0"
  },

And after that, this would work just as well:

require('paper');

But the shim packages are still useful for easer dependency management and updating...

kumarharsh commented 7 years ago

I'm using paper-jsdom for all the reasons you underlined above, as well as because when the next guy comes in and looks at the code, he'd be hard-pressed to figure out where jsdom is being used in the project.

lehni commented 7 years ago

Yes it does make sense. Glad to hear it works for you, too! : ) And updating happens automatically now through a Gulp tasks that keeps the versions in sync and publishes along with the main library, so it's not an additional managing effort.

lehni commented 7 years ago

While I was at it, I also streamlined the whole publishing process, which is now fully automated through Gulp. This also handles changes to the http://paperjs.org/ website, to which it now automatically pushes the new references, ZIP package and JS asset. And https://github.com/paperjs/paperjs.org now has a Travis CI worker that deploys the site statically to Github Pages at https://github.com/paperjs/paperjs.github.io every time a push happens to it. So releasing a new version unravels a sequence that results in everything being up to date.

Pure magic!

kumarharsh commented 7 years ago

Sweet