Update to latest createjs libs, latest canvas, working on recent node version#3
Update to latest createjs libs, latest canvas, working on recent node version#3jedateach wants to merge 9 commits intoCreateJS:masterfrom
Conversation
| var fs = require('fs'); | ||
| var Canvas = require('canvas'); | ||
| var Image = Canvas.Image; | ||
| var { Canvas, Image } = require('canvas'); |
There was a problem hiding this comment.
The canvas API changed in version 2.
Using this destructing syntax will break older versions of node. We can degrade it back to older syntax, if needed. But really a decision needs to be made on the minimum node version, and subsequently it should run on a CI test matrix that includes includes that minimum version of node.
| /** | ||
| * Helper for reporting back exec errors | ||
| */ | ||
| function execCallback(error, stdout, stderr) { |
There was a problem hiding this comment.
This helped reveal a problem in ffmpeg output. The cli tool has probably changed a bit since this work was originally done.
| "easel" | ||
| ], | ||
| "homepage":"https://github.com/wdamien/node-easel", | ||
| "repository":"git://github.com/wdamien/node-easel", |
There was a problem hiding this comment.
These urls are published on the npm registry: https://www.npmjs.com/package/node-easel
...which made it confusing that the main repo is actually under the CreateJS github organisation.
|
Thanks for this, it saved me tons of time researching how to get it to latest version. |
In short this PR:
canvaspackage to version 2easeljsvia npm, which gets required by the main file, and also gets served in the examples server.I chose to call my branch
1.0, because if you follow semantic versioning, then this is a breaking change to existing projects. Accordingly, I've bumped the package.json's version number to1.0.0.I've broken this work up into a few commits, but can squash it all down, if desired.
In doing this work, I realised there have actually been a few others work on the same issues. I've had a quick look through, and their changes do appear to be covered by this PR.