Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ESM support #186

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion nacl-fast.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
var nacl = {};
Copy link
Owner

@dchest dchest Jan 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This unfortunately pollutes global scope. Perhaps, I should move all this modularization stuff into a a build step, like I did here: https://github.com/dchest/fast-sha256-js

Copy link
Author

@SvanteRichter SvanteRichter Jan 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite understand what you did in fast-sha-256-js though. I thought exports in ESM were reserved for the top level as they are statically resolved, not dynamic. Would that still work as a direct import in a browser?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, they are for CommonJS, but this can also be used when directly included using <script> tag.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant for ESM (edited my comment before I saw you reply), can ESM export be dynamic or non-top-level?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, no I don't think it can be dynamic.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My hope is to be able to use this in a browser without any build step, so if that is possible while still keeping test and code duplication at an okay amount that'd be awesome.

If not I'll make it so some other way 🙂
Thanks for looking at and considering this!

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, thanks! I think I'll make a special build step that will insert those things and generate .mjs files.


(function(nacl) {
'use strict';

Expand Down Expand Up @@ -2388,4 +2390,4 @@ nacl.setPRNG = function(fn) {
}
})();

})(typeof module !== 'undefined' && module.exports ? module.exports : (self.nacl = self.nacl || {}));
})(typeof module !== 'undefined' && module.exports ? module.exports : typeof self !== 'undefined' ? (self.nacl = self.nacl || {}) : nacl);
Loading