asmcrypto / asmcrypto.js

JavaScript Cryptographic Library with performance in mind.
MIT License
659 stars 182 forks source link

ES6 modules #119

Closed alippai closed 6 years ago

alippai commented 7 years ago

Should we change the build to ES2015 modules + rollup?

alippai commented 7 years ago

Also the wrapper to use ES6 classes

vibornoff commented 7 years ago

Does it compatible with old browsers: IE10, Safari 5.1?

jdonaldson10 commented 7 years ago

@alippai I actually spent a couple of hours working on this yesterday and have it about 85% of the way there.

I've tried to do this with minimal disruption to the code. As such there isn't any es6 class syntax yet but that could be added if we want. Also the current */exports.js could probably be refactored away.

I've tended to towards default export to keep the public api/namespaceing isolated within the index.js. If others want a custom build they can then easily import in the specific functions they need direct from source and let their project's module builder handle it all. I've also favored import with destructing where possible to help maximize tree-shaking.

Doing this probably solves #88, #102, #112 and removes the need for the OpenPGP.js team's separate fork as per #95. More importantly it removes the need for the massive dependency list in the current gruntfile.js as every module describes it's own dependencies :ok_hand:

I can likely have something PR worthy by the weekend if that's acceptable?

So far:

Bundle Size

TODO

What do you think about adding some code coverage with istanbul and code quality stuff with eslint? Maybe best leaving those to separate issues as they could be quite time consuming to implement be quite impacting on the codebase.

alippai commented 7 years ago

The rollup can produce iife and amd/commonjs code. The classes are not supported only after transpiling with Babel.

Both improvements help the developer experience and IDE support.

On Wed, Mar 15, 2017, 15:46 Artem S Vybornov notifications@github.com wrote:

Does it compatible with old browsers: IE10, Safari 5.1?

โ€” You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/vibornoff/asmcrypto.js/issues/119#issuecomment-286764311, or mute the thread https://github.com/notifications/unsubscribe-auth/AAOsWS9KBvvBNfpUwSJ_SilTlPUfvxVcks5rl_nFgaJpZM4Md3CM .

alippai commented 7 years ago

Awesome!

jdonaldson10 commented 7 years ago

@vibornoff yes. By having having the bable transpile step, the output is an ES5 compatible UMD as per the current setup

vibornoff commented 7 years ago

BTW, why you guys don't use modern WebCrypto API?

alippai commented 7 years ago

I use it for streaming SHA, buggy Firefox crypto in FF45 (ESR) and pbkdf, rsa pss (missing in IE11)

On Wed, Mar 15, 2017, 16:13 Artem S Vybornov notifications@github.com wrote:

  • rollup
  • babel
  • grunt
  • gulp in place of grunt?
  • karma with saucelabs.com runner
  • add nodejs runner for tests (some folks use asmCrypto inside nodejs)

BTW, why you guys don't use modern WebCrypto API?

โ€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/vibornoff/asmcrypto.js/issues/119#issuecomment-286772871, or mute the thread https://github.com/notifications/unsubscribe-auth/AAOsWeadEWg0LdiTCF6Kkl_JVqgD9d6Nks5rl__VgaJpZM4Md3CM .

jdonaldson10 commented 7 years ago

I'm not convinced we need any task runner really. Some simple npm scripts to orchestrate our module bundler/test runner should be enough and save an extra dependency

// package.json
{
  /* ... */
  "scripts": {
    "prebuild": "eslint lib test",
    "build": "rollup -c",
    "watch": "rollup -c -w",
    "pretest": "npm run build",
    "test": "karma .....",
    "prepublish": "npm test"
  }
  /* ... */
}

As for webcrypto, yea I'm with you on that one. More of a side interest and some awkward corner cases.

vibornoff commented 7 years ago

Ok guys, I'd be glad to see your PRs! ๐Ÿ‘

alippai commented 7 years ago

@jdonaldson10 can you push your changes so I can help with testing and further work?

jdonaldson10 commented 7 years ago

Been a little swamped this past couple of weeks so haven't progressed as much as I'd hoped. In saying that I have the tests all passing locally.

Hopefully I can get sometime later today/tomorrow to bring it together

On 1 Apr 2017, at 23:22, รdรกm Lippai notifications@github.com wrote:

@jdonaldson10 can you push your changes so I can help with testing and further work?

โ€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

alippai commented 7 years ago

@jdonaldson10 I'd be happy if I could help your work in any way. Ping me, if you need help testing or in development.