Convert to ES modules #199
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is a somewhat major reworking of how the package is organized and tested. It's pretty opinionated so my expectation is not necessarily to have it merged, so much as to discuss the general approach and potential problems. Below is a list of some of the most significant issues I came across and changes I made while getting this working.
To keep complexity manageable, I propose eliminating all cjs and bumping a major version. Cjs folks can use the previous version, and important security fixes can be backported. Are there any good reasons to go through the trouble of trying to support cjs and mjs simultaneously?
Rename nacl-fast.js -> index.js and nacl.js -> original.js. I find nacl-fast.js being the main file confusing... maybe that's just me, but having the main file be called index.js seems more standard?
Move stuff that's not part of the test suite out to top-level folders (bench and timing).
Remove some stuff:
Eslint doesn't support dynamic imports yet and this doesn't appear to be something that can be easily worked around - I pulled in the babel-eslint parser which seems to do the trick for now.
Tape (the library used for testing) is not itself an es module and so it can't be used in the browser without a build step. To work around this, tap-esm has been used instead.
Browser tests no longer require a build step, but es modules do require an http server (I pulled in ecstatic for this purpose). Query params can be used to control which tests are run, and which variant of the library to use.
Use import maps in the browser to handle bare import specifiers. This feature has not been implemented by browsers yet, but there is a spec and a shim, which has been used here for the time being.
Use package.json exports field to indicate the main file and enable self-referencing import statements. E.g.
import nacl from 'tweetnacl'
from within the package itself.