dchest / tweetnacl-js

Port of TweetNaCl cryptographic library to JavaScript
https://tweetnacl.js.org
The Unlicense
1.77k stars 293 forks source link

Add ESM support #186

Closed SvanteRichter closed 4 years ago

SvanteRichter commented 4 years ago

This is a pretty small change (looks large because it includes the generated files) that adds an export into ESM (also known as es6 modules).

I couldn't figure out where the minified build jobs are triggered, but the build-esm job should probably be added there too (or is it done manually when a release is done?).

I think this meets the minimum to fix #170.

I didn't think this was large enough the warrant an AUTHORS.md inclusion, but let me know if that is needed/warranted.

It can probably be done a lot better in the future, but this was the least invasive way I could think of.

dchest commented 4 years ago

Awesome, thanks!

Yes, everything is built manually.

Is there a way to test it? Manually, or in an automated way.

SvanteRichter commented 4 years ago

Is there a way to test it? Manually, or in an automated way.

I've been testing it by manually modifying the tests mostly, I didn't want to be too invasive and changing the whole testsuite. For example this for a hash test:

import nacl from './nacl-fast.mjs';
import util from 'tweetnacl-util';
import test from 'tape';
nacl.util = util;

var enc = nacl.util.encodeBase64,
    dec = nacl.util.decodeBase64;

let randomVectors = /* insert/import ./data/hash.random.js here */

test('nacl.hash random test vectors', function(t) {
  randomVectors.forEach(function(vec) {
    var msg = dec(vec[0]);
    var goodHash = dec(vec[1]);
    var hash = nacl.hash(msg);
    t.equal(enc(hash), enc(goodHash));
  });
  t.end();
});

I think the problem is how to keep CommonJS compatability while still running tests on ESM files, especially when tests themselves include certain dependencies. One solution would be to build separate versions and then run tests against those, but that sort of defeats the purpose of having native modules.

Please let me know if you have an idea on how I can keep compatibility but still test ESM though, that'd be great :)

dchest commented 4 years ago

Thanks, I'll figure something out for testing.

Saiv46 commented 4 years ago

Any progress?

SvanteRichter commented 4 years ago

Closing since this is probably not getting merged and there are probably better (but also more invasive) ways to do it.

rozek commented 9 months ago

Thank you very much for your changes - although they did not make it into the original code, they still allowed me to make tweet-nacl.js ESM compatible in my own fork within a few minutes!

SvanteRichter commented 9 months ago

@rozek No problem, nice to see somebody using it! Seems like there are a couple of different forks that have done this now, maybe list yours also on https://github.com/dchest/tweetnacl-js/issues/170.