anseki / plain-draggable

The simple and high performance library to allow HTML/SVG element to be dragged.
https://anseki.github.io/plain-draggable/
MIT License
765 stars 96 forks source link

Add tips for the "losted" dependencies . #25

Closed niyan-ly closed 5 years ago

niyan-ly commented 5 years ago

Hi, @anseki . Thank you for creating such an awesome library, I've read several dependencies-related issues. I think before we find a better solution, it is better to add a section in the README to indicate that 'modern' users should install the losted dependencies by themselves. Otherwise, it is quiet confusing and may cause a time-consuming debugging.

anseki commented 5 years ago

Hi @hapulb, thank you for your comment. So, this library has no dependencies to run. To use this, you have to execute this only:

npm i plain-draggable
niyan-ly commented 5 years ago

But actually, the esm module do require some extra packages. And when using along with webpack(which you mean the 'modern' way), it complains that some modules cannot be resolved. I know that i could solve that manually by installing packages which is required but not in your dependencies list. And since this behavior is quiet unique to others, i here to ask maybe you can add some special hints to make other guys realizing this.

anseki commented 5 years ago

Generally speaking, developers that use Webpack require libraries to be bundled as development version. Then, the required additional packages for building are specified in PKG.devDependencies (dev means "development"). Those packages are installed automatically by npm install command.

This information is well known, and it is specification of NPM even if some developers don't know that. Therefore, I think that the information should be not added to the README at the moment because it is not required and to avoid confusion.

However, I'll rethink it when more requests come. Thank you for your proposal. :smile:

niyan-ly commented 5 years ago

Well, but i still want to say, that is, packages listed under dependencies field should be which is need to help current package works as intended. So we should list anim-event as one of the dependencies because plain-draggable can't work in production without it, but we do not need webpack and its tool chains in production so we just list them as devDependencies. So finally, when users add your package via npm i, npm will only install packages listed under the dependencies field, using --save-dev or -D just means adding your package into the devDependencies field of user's project, but any dependencies under your package's devDependencies will still be ignored. So how could i install those required dependecies manually? I have to add them to my project's dependencies or cd to your package under node_modules to execute npm i. And this is the Spec of NPM, no offense. And thanks again for your patience.

anseki commented 5 years ago

The already bundled packages never should be added to PKG.dependencies. Also, the PKG.devDependencies packages are installed automatically by npm install command, but current version of NPM doesn't support installing a package and its PKG.devDependencies packages at one time, as you said. This is being discussed at times. A future version of NPM might support that.

Anyway, that information should be not added to libraries now, as with almost libraries. Then, I close this issue for now. I will rethink it when more requests come. I still welcome comments. Thank you again. :smile:

benkingcode commented 5 years ago

This library does not work out of the box... there are a ton of missing dependencies. Why has this issue been closed?

anseki commented 5 years ago

Hi @dbbk, thank you for your comment. Please see: https://github.com/anseki/plain-draggable/issues/25#issuecomment-484334936

benkingcode commented 5 years ago

Thank you... I know how to install npm packages. Your package does not declare its dependencies, it is missing anim-event cssprefix m-class-list pointer-event. It seems like this issue keeps being raised by people, have you actually tried installing your own package?

anseki commented 5 years ago

This library has no dependencies to run. Please see this again: https://github.com/anseki/plain-draggable/issues/25#issuecomment-484334936 You don't have to install those three packages to run because those were already bundled in this package.

Those packages are required only in development. If you don't know "development" or PKG.devDependencies, please see document of NPM.

benkingcode commented 5 years ago

You keep saying that but it is not correct. Please just try and install your own project and use it.

It is plain to see if you just look at the code: https://github.com/anseki/plain-draggable/blob/master/plain-draggable.esm.js

The ESM version of plain-draggable is importing these packages, as you can see here: https://github.com/anseki/plain-draggable/blob/455d48dc0369c37d10d7e07cd8a2a33c12f1a1c8/plain-draggable.esm.js#L19

...but because they are listed as devDependencies in package.json, they do not get installed when someone installs and tries to use plain-draggable.

anseki commented 5 years ago

For example, see this page: https://anseki.github.io/plain-draggable/ You will see that those examples work fine. Then check that source code, and confirm that the three packages are not loaded. Then, you should understand that those are not required.

One of your mistake is that you tried to use a file that is not bundled file. Maybe you are trying to use the library with a development tool such as Webpack or another bundler. Of course those development tools require the development version such as ESM files. And, for development, the PKG.devDependencies packages are installed automatically by npm install command.

As I said, you don't have to install those three packages to run because those were already bundled in this package. Also, the bundled file is loaded via PKG.main. See: https://github.com/anseki/plain-draggable/blob/master/package.json In other words, only the PKG.main file is required to run, not plain-draggable.esm.js. That file for development, and the packages are required only in development.

I have to say again, please see document of NPM for "development" and PKG.devDependencies. Also, you have to understand the import mechanism (e.g. ESM) and the bundler. This is important point for beginners in JS, NPM, Webpack, etc. And you have to learn that before you try to use any library in development.

@poor-branson proposed adding information for developers. Of course @poor-branson understand "development" and PKG.devDependencies.

benkingcode commented 5 years ago

I'm not a beginner, I have 13 years of experience. I understand JavaScript, Webpack and NPM fine.

From the sounds of it you seem confused about the ESM module, is your point that nobody should use this? If so you should remove it from module and jsnext:main in package.json, because this is what Webpack is importing when it looks at your package.

Again, I would urge you to please just try importing your own library in a new Webpack / Rollup / whatever project, and see that it crashes, rather than proclaiming that everyone is just a beginner who doesn't understand how to install things.

anseki commented 5 years ago

Sorry, you are not beginner, but maybe you are misunderstanding the mechanism of the import (e.g. ESM) and the bundler. Why you use "ESM" file? Maybe you don't think that you are trying to use the library in development version. The development tools such as Webpack require the development version such as ESM files, it is understandably not bundled yet. Did you try the PKG.main file? Not in development, the PKG.main file that was already bundled is used. (i.e. you don't have to install the three packages.)

Or, do you mean that the three packages were not installed by npm install command?

benkingcode commented 5 years ago

I'm not explicitly choosing to use the ESM file, it's Webpack that is selecting it because as I mentioned it's declared as the module and jsnext:main in package.json

anseki commented 5 years ago

The development tools such as Webpack require the development version such as ESM files. You have to understand this.

benkingcode commented 5 years ago

Well no, they don't require ESM, but nonetheless if they use the ESM build it doesn't work because, as mentioned earlier, the dependencies used by this package are listed as devDependencies and thus do not get installed when a user installs this package.

It's really not complicated, I think you're confused along one or more lines... I'm just going to fork the package and fix it for myself.

anseki commented 5 years ago

I think that the tips is not needed for now because almost developers know the PKG.devDependencies packages are installed automatically by npm install command. However, of course you can customize it if you don't like typical package.

Anyway, thank you for your comments. :smile:

benkingcode commented 5 years ago

almost developers know the PKG.devDependencies packages are installed automatically by npm install command

I genuinely have no idea what you're talking about. This doesn't happen. If it happened the damn thing would work out of the box. I give up.

anseki commented 5 years ago

Sorry, my English is poor. I mean, generally speaking, developers use the npm install command to install the PKG.devDependencies packages.

I already asked you: https://github.com/anseki/plain-draggable/issues/25#issuecomment-488243929 Do you mean that the three packages were not installed by npm install command? If so, your NPM might be broken, or your computer might have a problem.