neocotic / qrious

Pure JavaScript library for QR code generation using canvas
https://neocotic.com/qrious
Other
1.55k stars 215 forks source link

npm registry still lists node-canvas as dependency #68

Closed neocotic closed 7 years ago

neocotic commented 7 years ago

It has been noted by @justin-lau on #28 that, although the package.json file in master only declares node-canvas under optionalDependencies, the npm registry appears to have it listed under optionalDependencies and dependencies:

qrious (master) λ npm view qrious@2.2.0 dependencies
{ canvas: '^1.3.12' }
qrious (master) λ npm view qrious@2.2.0 optionalDependencies
{ canvas: '^1.3.12' }

This is very odd and I'm not sure why it's doing that. I think the first, and possibly easiest, attempt to fit it would be to simply attempt a bump of the fix version to 2.2.1. This should hopefully give npm the nudge it needs (for some reason). If that doesn't fix it, I'll have to reach out to npm for further assistance.

khawarizmus commented 7 years ago

The moment i try to import qrious i get this error which i guess is related to this issue. How can i fix this? Imgur

neocotic commented 7 years ago

@gimyboya Well, that depends. I'll need more context about how you're using QRious.

I see that you're importing the CommonJS version, so I suspect that you're developing in Node.js?

Currently, QRious has 2 distributions (3 if you include the minified version):

If you're developing for the Node.js runtime, you'll almost certainly need to install canvas. If developing for the browser runtime, you should be using the UMD distribution which relies on native HTML5 canvas.

khawarizmus commented 7 years ago

@neocotic I am currently developing on electron with node and I used both syntax commonjs and ES6 imports both returns the same error.

So is the only solution is to install canvas?

neocotic commented 7 years ago

@gimyboya CommonJS and ES6 imports (sort of, see note at bottom of comment) both only work for Node.js environments with the current setup. I am considering splitting this project up to simplify this whole thing (see #53) as I for one don't like the whole optional dependency situation with canvas as a solution to browser's not requiring it.

I'm hoping to get around to it soon but I need to address the code base first as the file sizes are too big. Current thought is to do away with transpiling the code, as that seems better suited to applications than libraries when it comes to bundle sizes as I'm having to include babel-runtime. But I'm getting carried away.

WRT to your question about whether you just need to install canvas; I'm not familiar enough with electron to say for sure. I understand that electron allows you develop desktop apps based on Chromium and Node.js so I guess it just depends on where you plan on using QRious within your electron code base. The simplest answer is that, if HTMLCanvasElement exists in the global scope and you can do document.createElement('canvas') in the location of the code where you want to use QRious, you should use the browser version. Otherwise, it looks like you'll need to use the Node.js version. That said; if electron handles things totally differently. I could consider creating a plugin to support it after I've split up the code base.

Although I said earlier that ES6 imports work only for Node.js environments; I imagine you should be able to use it if you're specifically targetting the browser runtime file. For example;

import QRious from 'qrious/src/runtime/browser';
neocotic commented 7 years ago

I only mentioned babel because, if I choose to drop that, I'll also be dropping support for ES6 modules. Source code would be written in ES5 using CommonJS and then bundled when needed (e.g. UMD for browser).

khawarizmus commented 7 years ago

@neocotic Thanks for your answer i will try :

import QRious from 'qrious/src/runtime/browser';

and i will let you know the result also i just realised that i installed it with npm install --save --no-optional qrious which is why it's breaking... if that don't work i will install it normally

khawarizmus commented 7 years ago

nothing is working for node.... I tried this

import QRious from 'qrious/src/runtime/browser';

and i tried to do npm install --save qruios still got the same problem and when i tried to install canvas with npm install canvas and npm install canvas@1.6.5 couldn't install it and it seems messed up many users are facing problems installing it so i doped it.

finally i solved it by using it just like i would in a browser... but this means that it will be painful to update it later

neocotic commented 7 years ago

@gimyboya I'm confused again :confused:

I still don't know how you plan on using it or what environment it is running in (i.e. is DOM available, or is it in Node.js module?). You could try importing the browser distribution:

import QRious from 'qrious/dist/umd/qrious';

But, again, I'm guessing here as I don't have enough information to help you.

If anything, you've helped me decide that I should indeed go ahead with splitting this project up into modules so that Node.js and browser implementations are separate. I'll try to get this done soon in a major release (since it's a breaking change) but only after I've sorted out how the code is written (which will probably be another major release as I'll most likely no longer be using ES6 modules any more, which could be a breaking change).

khawarizmus commented 7 years ago

@neocotic Electron is a bit special it's like a browser with node so i can use both and electron-forge uses electron-compile under the hood so i can even extend the browser capacity to use ES6 syntax and sass or less or whatever directly on the fly.

If anything, you've helped me decide that I should indeed go ahead with splitting this project up into modules so that Node.js and browser implementations.

please go ahead

(which will probably be another major release as I'll most likely no longer be using ES6 modules any more, which could be a breaking change).

i would highly recommande that also because electron compile can not compile node_modules folder and it will break it's browser if it meets a non supported ES6 syntax such as imports.

neocotic commented 7 years ago

Back to the original point of this issue, it looks like npm seems to simply include optional dependencies under dependencies as well, so I'm going to close this issue.

Since I'm almost certainly going to split this project out (see #53), this issue should resolve itself as the current qrious project will be only for browser, so the canvas dependency will be moved to the new qrious-node module (or whatever I choose to call it). If it still exists after that, I'll reach out to npm as there's clearly a bug then.