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

Remove canvas dependency #194

Merged
merged 6 commits into from
Sep 11, 2015

Conversation

Rayman
Copy link
Contributor

@Rayman Rayman commented Sep 1, 2015

The canvas dependency makes this library difficult to install on for example travis and windows. This PR removes that dependency such that no native Node.js extensions are needed (no more libcairo2-dev libjpeg8-dev libpango1.0-dev libgif-dev build-essential g++, just JavaScript code).

I modified the logic a bit but I think I tested all the code paths. Maybe we need CI tests to check the library with BSON and/or png compression.

  • in a browser a canvas element will (still) be used for decoding pngs (no
    external dependencies needed)
  • in node it will use the pngparse module which will use the native zlib
    module to inflate the data

@@ -40,7 +41,8 @@
"canvas": "./src/util/shim/canvas.js",
"eventemitter2": "./src/util/shim/EventEmitter2.js",
"ws": "./src/util/shim/WebSocket.js",
"xmldom": "./src/util/shim/xmldom.js"
"xmldom": "./src/util/shim/xmldom.js",
"../util/decompressPng": "./src/util/shim/decompressPng"
Copy link
Contributor

Choose a reason for hiding this comment

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

should it be ./src/util...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's what I also thought but this is the way aliasify works apparently 😞 We could rewrite it using the browser field of the package.json and not use aliasify.

This commit makes sure that
- in a browser a canvas element will be used for decoding pngs (no
  external dependencies needed)
- in node it will use the pngparse module which will use the native zlib
  module to inflate the data
@Rayman Rayman force-pushed the remove-canvas-dependency branch from a148bb5 to 1cb6e01 Compare September 9, 2015 13:33
@Rayman
Copy link
Contributor Author

Rayman commented Sep 9, 2015

Fixed the merge conflict and removed aliasify so that it is more clear how the shims work.

rctoris added a commit that referenced this pull request Sep 11, 2015
@rctoris rctoris merged commit 1ca42c2 into RobotWebTools:develop Sep 11, 2015
@Rayman Rayman deleted the remove-canvas-dependency branch September 14, 2015 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants