photopea / UZIP.js

Simple ZIPping library for Javascript
MIT License
187 stars 27 forks source link

Add support install via npm or yarn #6

Closed mike667 closed 5 years ago

mike667 commented 5 years ago

Hello. Can add support install via npm or yarn?

Thank you.

photopea commented 5 years ago

Hi, I never "installed" javascript libraries. Would you like to put UZIP.js on npm? Do you think you can make npm load a new version from GitHub every time we update it?

igandrews commented 5 years ago

UZip is on npm at this point but it hasn't been updated with recent changes. Since @photopea is listed as a collaborator can you publish an updated version there?

photopea commented 5 years ago

I went to npm.org to the UZIP project and logged in. Is there a button "get update from Github", or a way to upload a new version manually?

I really don't want to download and install megabytes of their software, just to send 25 kB of data somewhere :(

igandrews commented 5 years ago

I don’t think there is anything to do that. I think the steps are using npm publish as mentioned here which requires installing npm which requires node. Unless you contact the person who put it there and get them to update it?

igandrews commented 5 years ago

I think I see now. It looks like sheetjs submitted a PR to your repo for adding a package.json and a module export line at the end (and lots of whitespace changes). It was that version that they published to npm and they mentioned that in the PR.

SheetJSDev commented 5 years ago

About the npm package uzip: As mentioned in the PR it is literally a one-line addition to make photopea work in nodejs and bundlers without disrupting anything else:

if(typeof module !== 'undefined' && typeof module.exports !== 'undefined') module.exports = UZIP;

We published the repo from our fork (the other changes were trailing whitespace removal). In addition, we added the npm user photopea as a maintainer so that, if the author does decide to add support, he or she can continue to maintain the module.

@photopea assuming you still have control over the photopea user, please consider adding the line and publishing releases. That would make it much easier to consume the library with minimal effort on your end. If you lost access to that account but would like to control publishing the module, it's easy to add another maintainer.

@igandrews publishing served as a stop-gap measure, ensuring that users had access to something and that the name uzip on npm would be available if @photopea decided to publish a module. It was never our intention to maintain it. It's easy to transfer the module to someone who is interested in keeping it up to date. Are you ~igandrews on npm or should we transfer to another account?

photopea commented 5 years ago

I can add the line into the source code. But don't you mind, that my current library creates a global variable UZIP? Do you want me to wrap it into anonymous function, which either sets module.exports, or window.UZIP?

BTW. I maintain UZIP.js only here on GitHub. If you want it anywhere else (e.g. on npm), feel free to upload it there.

SheetJSDev commented 5 years ago

CODE:

In NodeJS, the code is wrapped in a function call:

(function(exports, require, module, __filename, __dirname) {
    ....
});

So the existing var UZIP = {} is not global from the perspective of nodejs. Similarly, the webpack wrapper also hides the code within a function that ensures UZIP isn't leaked as a global.

You can use a wrapper and test for window/global if you like but it is not necessary, things will "just work" :)

NPM:

It's possible to set up a github flow (using a "continuous delivery" service that supports github, like codeship) that will automatically publish to npm every time you push. Until that is set up, we or someone else could maintain a cron job that pulls once a day and publishes to npm.

Since you aren't using version numbers (would you consider using semver?), the process could use a version number scheme like 0.<YYYYMMDD>.<HHMMSS>.

photopea commented 5 years ago

Ok I added the line which sets module.exports . Is it appropriate for the npm now?

SheetJSDev commented 5 years ago

Yes this is sufficient. You can actually test it, if you have node installed, by running node -pe "require('./uzip')" from your directory.

We just published 0.20191010.0 using the script https://gist.github.com/SheetJSDev/e2f904c6588789e453359e1b02004dae (don't run it locally, it will remove your existing UZIP.js directory). There are a few adjustments to be made to the generated package.json, but at this point you don't need to do anything else :)

SheetJSDev commented 5 years ago

Ok so the process works and is in place, there will be daily updates, and this issue can be closed.

photopea commented 5 years ago

Hi, so the latest UZIP.js is on npm now, and README cotnains the details about installing it.