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

Use canvas-prebuilt instead of canvas in order to drop external dependencies #181

Merged
merged 3 commits into from
Oct 20, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 0 additions & 14 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,24 +1,10 @@
sudo: required
services:
- docker
language: node_js
node_js:
- '8'
- '9'
cache:
directories:
- node_modules
addons:
apt:
packages:
- gcc-4.8
- g++-4.8
- libcairo2-dev
- libjpeg8-dev
- libgif-dev
- libpango1.0-dev

env: CXX="g++-4.8" CC="gcc-4.8"

script: "npm run-script travis"
after_success: "<coverage/lcov.info ./node_modules/coveralls/bin/coveralls.js"
Expand Down
13 changes: 0 additions & 13 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -160,19 +160,6 @@ Color Module</a>, as far as I can tell. Defaults to `transparent`.
Installation
------------

For creating the sprite images themselves AssetGraph-sprite uses <a
href="http://github.com/LearnBoost/node-canvas">node-canvas</a>, which
is not a pure-node module and requires the Cairo development sources
(version 1.10 or later), `libjpeg` (version 8 or later) and
`libgif`. On Ubuntu 10.10 and above you should be able to get them
like this:

```
$ sudo apt-get install libcairo2-dev libgif-dev libjpeg8-dev
```

Now you can proceed to install AssetGraph-sprite:

```
$ npm install assetgraph-sprite
```
Expand Down
29 changes: 9 additions & 20 deletions lib/spriteBackgroundImages.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
const queryString = require('querystring');
const { promisify } = require('util');
const packers = require('./packers');
let Canvas;
const canvasPrebuilt = require('canvas-prebuilt');

try {
Canvas = require('canvas');
} catch (e) {}
const Canvas = canvasPrebuilt.Canvas;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could use destructuring?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. It's that ok with our node version support?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Destructuring assignment is node 6.5.0+ so yeah. We should also lebab/prettierify the code a bit :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting ready to introduce prettier etc. over here: #182

const Image = canvasPrebuilt.Image;

// Helper for extracting all nodes defining a specific property from a postcss rule
function getProperties(container, propertyName) {
Expand All @@ -15,10 +14,14 @@ function getProperties(container, propertyName) {
}

async function getCanvasImageFromImageAsset(imageAsset) {
const canvasImage = new Canvas.Image();
const canvasImage = new Image();
await new Promise((resolve, reject) => {
canvasImage.onerror = err => {
err.message += ' (' + imageAsset.urlOrDescription + ')';
if (err.message.includes('node-canvas was built without SVG support')) {
err.message = 'Adding SVG images to a sprite is not possible';
}

err.message += ` (${imageAsset.urlOrDescription})`;
reject(err);
};
canvasImage.onload = resolve;
Expand Down Expand Up @@ -107,20 +110,6 @@ function extractInfoFromCssRule(cssRule, propertyNamePrefix) {

module.exports = () => {
return async function spriteBackgroundImages(assetGraph) {
if (!Canvas) {
assetGraph.warn(new Error('assetgraph-sprite: Canvas not found, skipping'));
return;
}

// Waiting for https://github.com/LearnBoost/node-canvas/issues/52
const cairoVersion = Canvas.cairoVersion.split('.').map(
str => parseInt(str, 10)
);
if (cairoVersion[0] < 1 || cairoVersion[1] < 10) {
assetGraph.warn(new Error('assetgraph-sprite: Cannot create sprites due to missing canvas.getContext("2d").drawImage() support. Please compile node-canvas with Cairo version 1.10.0 or above.'));
return;
}

const spriteGroups = {};

// Find sprite annotated images and create a data structure with their information
Expand Down
8 changes: 4 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@
"files": [
"lib"
],
"optionalDependencies": {
"canvas": "1.6.5"
},
"devDependencies": {
"assetgraph": "5.4.0",
"coveralls": "^3.0.0",
Expand All @@ -39,5 +36,8 @@
"coverage": "istanbul cover _mocha",
"travis": "npm run lint && npm run coverage"
},
"main": "lib/spriteBackgroundImages.js"
"main": "lib/spriteBackgroundImages.js",
"dependencies": {
"canvas-prebuilt": "2.0.0-alpha.14"
}
}
2 changes: 1 addition & 1 deletion test/spriteBackgroundImages.js
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,6 @@ describe('spriteBackgroundImages', function () {

expect(assetGraph, 'to contain asset', 'Svg');

await expect(assetGraph.queue(spriteBackgroundImages()), 'to be rejected with', /error while reading from input stream/);
await expect(assetGraph.queue(spriteBackgroundImages()), 'to be rejected with', /Adding SVG images to a sprite is not possible/);
});
});