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

[1.3] Build system bug? (canvas, konva, jsdom) #6613

Closed
ffxsam opened this issue Mar 29, 2016 · 12 comments
Closed

[1.3] Build system bug? (canvas, konva, jsdom) #6613

ffxsam opened this issue Mar 29, 2016 · 12 comments

Comments

@ffxsam
Copy link
Contributor

ffxsam commented Mar 29, 2016

This is a weird one. I can't even really explain it, but things are majorly broken.

Here's how to replicate:

  1. git clone https://github.com/ffxsam/ffx-meteor-react-boilerplate.git weird-bug
  2. cd weird-bug
  3. npm install && npm run scaffold
  4. npm install --save konva canvas (see note below)
  5. meteor

(note: make sure cairo is installed! brew install cairo)

At this point, everything should be fine. Now, break it. Edit /client/pages/Home.js and add this line below the import React line:

import Konva from 'konva';

Save it. Then this happens:

Unable to resolve some modules:

  "../build/Release/canvas" in
/Users/samh/weird-bug/node_modules/canvas/lib/bindings.js (web.browser)

Any ideas?

@ffxsam
Copy link
Contributor Author

ffxsam commented Mar 29, 2016

As a hack, if I go into /node_modules/canvas/build/Release and do:

ln -s canvas.node canvas

Then the app runs with no errors. Chrome works great. Firefox, not so much:

SyntaxError: invalid for/in left-hand side modules.js:75829:5

which appears to be this code coming from jsdom:

// Proxy feature functions to features module.                                                                         // 56
for (const propName of ["availableDocumentFeatures", "defaultDocumentFeatures", "applyDocumentFeatures"]) {            // 57
  defineGetter(exports, propName, () => documentFeatures[propName]);                                                   // 58
  defineSetter(exports, propName, val => documentFeatures[propName] = val);                                            // 59
}

It doesn't like for (x of y). It's as if it's not being transpiled.

@benjamn
Copy link
Contributor

benjamn commented Mar 29, 2016

The Unable to resolve some modules warnings are just warnings, not fatal errors. Sometimes a module appears to import another module that doesn't exist, but the code for the import is never actually executed, so it's not a problem. I suspect ../build/Release/canvas is meant to refer to a .node binary module, and there's no way to load those modules on the web.browser platform anyway.

@ffxsam
Copy link
Contributor Author

ffxsam commented Mar 29, 2016

The weird thing is, I'm not sure why Konva (HTML5 canvas library) would refer to a binary anyway. I guess that's a separate issue.
(Ok, I actually did npm r --save canvas and things still work fine in Chrome. I Just get the warning below)

Unable to resolve some modules:

  "canvas" in
/Users/samh/waves/node_modules/jsdom/lib/jsdom/living/nodes/HTMLCanvasElement-impl.js
(web.browser)

Why is jsdom being so finicky? What about the errors in Firefox?

@benjamn
Copy link
Contributor

benjamn commented Mar 29, 2016

Firefox is complaining about the const keyword I think. You may need to use an older version of jsdom: https://github.com/tmpvar/jsdom#install

@ffxsam
Copy link
Contributor Author

ffxsam commented Mar 29, 2016

Ahh.. so again, not a Meteor issue. :) Thanks for pointing me in the right direction, Ben!

@ffxsam ffxsam closed this as completed Mar 29, 2016
@DethCount
Copy link

Still a bug. The canvas package shouldn't be loaded by web.browser.
I never imported it on client side...

@omidahourai
Copy link

Bump. Everything worked fine for me with Meteor 1.3-beta8, but upgrading breaks canvas with the same warning/ error. More specifically, I'm using fabricjs with npm install fabric --save which has the canvas and jsdom dependencies.

@abernix
Copy link
Contributor

abernix commented Aug 4, 2016

If the canvas package should not be on the client, then it might be advisable for them to make some changes to their package.json to make this more clear to projects like Meteor who have to consume them. While they do specify the engine, they don't specify engineStrict so it merely acts as an advisory – but regardless, engineStrict has been deprecated in NPM 3.0 anyhow. Another (probably better solution) is the browser attribute (which they don't currently set) might solve the problem (though someone would have to test to be sure):

"browser": "./browser.js",

And then browser.js could just be an empty file right next to its package.json.

Just a thought!

@ykshev
Copy link

ykshev commented Aug 23, 2016

Have this issue. I'm trying to install meteor npm install --save canvas and get some warning during installation

meteor npm install --save canvas

> canvas@1.4.0 install /Users/Andrew/Sites/insper/node_modules/canvas
> node-gyp rebuild

  SOLINK_MODULE(target) Release/canvas-postbuild.node
  CXX(target) Release/obj.target/canvas/src/Canvas.o
  CXX(target) Release/obj.target/canvas/src/CanvasGradient.o
  CXX(target) Release/obj.target/canvas/src/CanvasPattern.o
In file included from ../src/CanvasPattern.cc:10:
../src/CanvasPattern.h:23:9: warning: private field '_width' is not used [-Wunused-private-field]
    int _width, _height;
        ^
../src/CanvasPattern.h:23:17: warning: private field '_height' is not used [-Wunused-private-field]
    int _width, _height;
                ^
2 warnings generated.
  CXX(target) Release/obj.target/canvas/src/CanvasRenderingContext2d.o
  CXX(target) Release/obj.target/canvas/src/color.o
  CXX(target) Release/obj.target/canvas/src/Image.o
  CXX(target) Release/obj.target/canvas/src/ImageData.o
  CXX(target) Release/obj.target/canvas/src/init.o
  CXX(target) Release/obj.target/canvas/src/FontFace.o
In file included from ../src/FontFace.cc:7:
../src/FontFace.h:27:15: warning: private field '_ftFace' is not used [-Wunused-private-field]
    FT_Face   _ftFace;
              ^
1 warning generated.
  SOLINK_MODULE(target) Release/canvas.node
insper@0.0.1 /Users/Andrew/Sites/insper
└── canvas@1.4.0 

after all on meteor start

Unable to resolve some modules:

  "../build/Release/canvas" in /Users/Andrew/Sites/insper/node_modules/canvas/lib/bindings.js (web.browser)

I've tried to add "browser": "./browser.js" to package.json, but it didn't help. Any ideas hot to fix? (METEOR@1.4.1)

@btbjosh
Copy link

btbjosh commented Oct 20, 2016

This canvas package thing is killing me, too.

@datacarl
Copy link
Contributor

Had the same issue as @tastemeru. Fixed it by cloning our repo again and then meteor npm install. I suppose it's some issue with upgrading.

@ghost
Copy link

ghost commented Nov 6, 2016

Deleting the node modules folder from the project and doing a clean install by meteor npm install fixed it for me @btbjosh. @datacarl seconded! i think so, too.

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

No branches or pull requests

8 participants