Refactor code to use ES module syntax#74
Conversation
- Changed all instances of `module.exports` to `export default` or named exports in various files including build scripts and utility functions. - Updated import statements to use ES module syntax across multiple test files and tools. - Introduced a new Vite configuration file for building the project, specifying library entry points and external dependencies. - Adjusted test files to use `beforeAll` instead of `before` for asynchronous setup. - Modified file writing operations to use ES module compatible methods.
- Updated test files (bn128.js, f1.js, fft.js, int.js, mnt6753.js, tomcook.js) to import Vitest's describe, assert, it, and beforeAll functions instead of using assert from "assert". - Improved formatting and consistency in the test files by adjusting spacing and indentation. - Modified buildpedersentablebases_mnt6753.js to export pedersen parameters as a constant instead of a default export. - Enhanced vite.config.js to support both unit and browser testing environments using Vitest with Playwright integration.
…de and improve code readability
There was a problem hiding this comment.
Pull request overview
This PR migrates the wasmcurves codebase from CommonJS to native ES modules, updates the test/build tooling to match (vitest + vite), and adjusts CI/lint configuration accordingly.
Changes:
- Convert runtime and build scripts from
require/module.exportstoimport/export, and set"type": "module"with updatedexportsmapping inpackage.json. - Replace Mocha-based test runner usage with Vitest (including async setup via
beforeAll) and add a Vite config for producing a CJS distribution artifact (build/main.cjs). - Replace legacy ESLint config with a flat
eslint.config.jsand update CI workflow steps for the new toolchain.
Reviewed changes
Copilot reviewed 51 out of 56 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| vite.config.js | Adds Vite build config for emitting a CJS library artifact and configures Vitest projects (node + browser). |
| tools/buildwasm_mnt6753.js | Converts build script to ESM and switches generated output to ESM exports. |
| tools/buildwasm_bn128.js | Converts build script to ESM and switches generated output to ESM exports. |
| tools/buildwasm_bls12381.js | Converts build script to ESM and switches generated output to ESM exports. |
| tools/buildpedersentablebases_mnt6753.js | Converts generator script to ESM and changes generated pedersen params module shape. |
| test/tomcook.js | Migrates test to ESM imports and Vitest lifecycle (beforeAll). |
| test/mnt6753.js | Migrates test to ESM/Vitest and changes pedersen params decoding approach. |
| test/int.js | Migrates test to ESM imports and Vitest lifecycle (beforeAll). |
| test/helpers/helpers.js | Converts helper module export to ESM named export. |
| test/fft.js | Migrates test to ESM imports and Vitest lifecycle (beforeAll). |
| test/f1.js | Migrates test to ESM imports and removes Mocha-specific timeout usage. |
| test/bn128.js | Migrates test to ESM/Vitest and normalizes formatting of arithmetic and loops. |
| test/bls12381.js | Migrates test to ESM/Vitest and normalizes formatting of arithmetic and loops. |
| src/utils.js | Converts exported utility functions to ESM named exports. |
| src/mnt6753/build_mnt6753.js | Converts curve builder module to ESM and updates internal imports. |
| src/f1.js | Converts module to ESM and updates imports/exports. |
| src/build_tomcook.js | Converts module to ESM default export and replaces assert-based guard with thrown Error. |
| src/build_timesscalarnaf.js | Converts module to ESM default export. |
| src/build_timesscalar.js | Converts module to ESM default export. |
| src/build_testg1.js | Converts module to ESM default export. |
| src/build_testf1.js | Converts module to ESM default export. |
| src/build_test.js | Converts module exports to ESM named exports. |
| src/build_qap.js | Converts module to ESM default export. |
| src/build_pol.js | Converts module to ESM default export. |
| src/build_pedersenhash.js | Converts module to ESM default export. |
| src/build_multiexp.js | Converts module to ESM default export. |
| src/build_mulacc.js | Converts module to ESM default export. |
| src/build_mem.js | Converts module to ESM default export. |
| src/build_int.js | Converts module to ESM default export. |
| src/build_fft.js | Converts module to ESM and updates utils/bigint imports. |
| src/build_f3m.js | Converts module to ESM and updates imports to include .js extensions. |
| src/build_f2m.js | Converts module to ESM and updates imports to include .js extensions. |
| src/build_f1m.js | Converts module to ESM and updates dependent imports/exports. |
| src/build_f1.js | Converts module to ESM and updates imports. |
| src/build_curve_projective.js | Converts module to ESM and updates scalar-builder imports. |
| src/build_curve_jacobian_a0.js | Converts module to ESM and updates internal imports. |
| src/build_batchop.js | Converts module to ESM default export. |
| src/build_batchinverse.js | Converts module to ESM default export. |
| src/build_batchconvertion.js | Converts module to ESM default export. |
| src/build_applykey.js | Converts module to ESM default export. |
| src/bn128/build_bn128.js | Converts curve builder to ESM and updates internal imports. |
| src/bls12381/build_bls12381.js | Converts curve builder to ESM and updates internal imports. |
| src/bigint.js | Converts bigint utilities to ESM named exports. |
| package.json | Declares ESM package type, adds Vite/Vitest tooling, and updates entrypoints/exports/scripts. |
| index.js | Converts package entry module to ESM exports. |
| eslint.config.js | Adds ESLint flat config and replaces removed legacy config/ignore files. |
| build/pedersenparams_mnt6753.js | Updates generated pedersen params module to export base64 payload as ESM. |
| .github/workflows/ci.yml | Updates CI to run on Ubuntu LTS Node and adds Playwright installation step. |
| .eslintrc.js | Removes legacy ESLint config in favor of flat config. |
| .eslintignore | Removes legacy ignore file (replaced by ignores in flat config). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| `export const code = "${Buffer.from(code).toString("base64")}"; | ||
| export const pq = ${moduleBuilder.modules.mnt6753.pq}; | ||
| export const pr = ${moduleBuilder.modules.mnt6753.pq}; | ||
| export const pG1gen = ${moduleBuilder.modules.mnt6753.pG1gen}; |
There was a problem hiding this comment.
Generator was fixed in 8ffaf38, but the committed build/mnt6753_wasm.js artifact was still stale (pr = 1352, equal to pq). Regenerated it — pr is now 48488.
| @@ -164,19 +162,19 @@ describe("Basic tests for g1 in bls12-381", function () { | |||
| assert(res2[1] = res3[1]); | |||
| }); | |||
There was a problem hiding this comment.
Fixed → assert.equal(res2[0], res3[0]) / assert.equal(res2[1], res3[1]). The real issue was = (assignment) instead of a comparison — a silent no-op assertion. Note Vitest's assert is callable (Chai interface), so that part of the concern does not apply; switching to assert.equal just matches the rest of the file.
| function printF1(s, p) { | ||
| console.log(s, " " + ns(p)); | ||
| } | ||
|
|
||
| //eslint-disable-next-line no-unused-vars | ||
|
|
||
| function printF2(s, p) { | ||
| console.log(s + " Fq2(" + ns(p) + " + " + ns(p+32) +"*u " ); | ||
| console.log(s + " Fq2(" + ns(p) + " + " + ns(p + 32) + "*u "); | ||
| } | ||
|
|
||
| //eslint-disable-next-line no-unused-vars | ||
|
|
||
| function printF6(s, p) { | ||
| console.log(s + " [Fq2(\n" + ns(p) + " +\n " + ns(p+32) +"*u],[" ); | ||
| console.log("Fq2(\n" + ns(p+32*2) + " +\n " + ns(p+32*3) +"*u],[" ); | ||
| console.log("Fq2(\n" + ns(p+32*4) + " +\n " + ns(p+32*5) +"*u]" ); | ||
| console.log(s + " [Fq2(\n" + ns(p) + " +\n " + ns(p + 32) + "*u],["); | ||
| console.log("Fq2(\n" + ns(p + 32 * 2) + " +\n " + ns(p + 32 * 3) + "*u],["); | ||
| console.log("Fq2(\n" + ns(p + 32 * 4) + " +\n " + ns(p + 32 * 5) + "*u]"); | ||
| } | ||
|
|
||
| //eslint-disable-next-line no-unused-vars | ||
|
|
||
| function printF12(s, p) { | ||
| console.log(s + " [ [Fq2(\n" + ns(p) + " +\n " + ns(p+32) +"*u],[" ); | ||
| console.log("Fq2(\n" + ns(p+32*2) + " +\n " + ns(p+32*3) +"*u],[" ); | ||
| console.log("Fq2(\n" + ns(p+32*4) + " +\n " + ns(p+32*5) +"*u]]" ); | ||
| console.log("[ [Fq2(\n" + ns(p+32*6) + " +\n " + ns(p+32*7) +"*u],[" ); | ||
| console.log("Fq2(\n" + ns(p+32*8) + " +\n " + ns(p+32*9) +"*u],[" ); | ||
| console.log("Fq2(\n" + ns(p+32*10) + " +\n " + ns(p+32*11) +"*u]]" ); | ||
| console.log(s + " [ [Fq2(\n" + ns(p) + " +\n " + ns(p + 32) + "*u],["); | ||
| console.log("Fq2(\n" + ns(p + 32 * 2) + " +\n " + ns(p + 32 * 3) + "*u],["); | ||
| console.log("Fq2(\n" + ns(p + 32 * 4) + " +\n " + ns(p + 32 * 5) + "*u]]"); | ||
| console.log("[ [Fq2(\n" + ns(p + 32 * 6) + " +\n " + ns(p + 32 * 7) + "*u],["); | ||
| console.log("Fq2(\n" + ns(p + 32 * 8) + " +\n " + ns(p + 32 * 9) + "*u],["); | ||
| console.log("Fq2(\n" + ns(p + 32 * 10) + " +\n " + ns(p + 32 * 11) + "*u]]"); | ||
| } |
There was a problem hiding this comment.
Lint was actually passing, but only because the flat config defined a rules: key in the same object that spread ...js.configs.recommended, clobbering the recommended ruleset — so no-unused-vars never ran. Fixed the config to apply recommended, and re-added the // eslint-disable-next-line no-unused-vars suppressions for these debug helpers (they were present before this PR).
| function printF1(s, p) { | ||
| console.log(s, " " + ns(p)); | ||
| } | ||
|
|
||
| //eslint-disable-next-line no-unused-vars | ||
|
|
||
| function printF2(s, p) { | ||
| console.log(s + " Fq2(" + ns(p) + " + " + ns(p+n8q) +"*u " ); | ||
| console.log(s + " Fq2(" + ns(p) + " + " + ns(p + n8q) + "*u "); | ||
| } | ||
|
|
||
| //eslint-disable-next-line no-unused-vars | ||
|
|
||
| function printF6(s, p) { | ||
| console.log(s + " [Fq2(\n" + ns(p) + " +\n " + ns(p+n8q) +"*u],[" ); | ||
| console.log("Fq2(\n" + ns(p+n8q*2) + " +\n " + ns(p+n8q*3) +"*u],[" ); | ||
| console.log("Fq2(\n" + ns(p+n8q*4) + " +\n " + ns(p+n8q*5) +"*u]" ); | ||
| console.log(s + " [Fq2(\n" + ns(p) + " +\n " + ns(p + n8q) + "*u],["); | ||
| console.log("Fq2(\n" + ns(p + n8q * 2) + " +\n " + ns(p + n8q * 3) + "*u],["); | ||
| console.log("Fq2(\n" + ns(p + n8q * 4) + " +\n " + ns(p + n8q * 5) + "*u]"); | ||
| } | ||
|
|
||
| //eslint-disable-next-line no-unused-vars | ||
|
|
||
| function printF12(s, p) { | ||
| console.log(s + " [ [Fq2(\n" + ns(p) + " +\n " + ns(p+n8q) +"*u],[" ); | ||
| console.log("Fq2(\n" + ns(p+n8q*2) + " +\n " + ns(p+n8q*3) +"*u],[" ); | ||
| console.log("Fq2(\n" + ns(p+n8q*4) + " +\n " + ns(p+n8q*5) +"*u]]" ); | ||
| console.log("[ [Fq2(\n" + ns(p+n8q*6) + " +\n " + ns(p+n8q*7) +"*u],[" ); | ||
| console.log("Fq2(\n" + ns(p+n8q*8) + " +\n " + ns(p+n8q*9) +"*u],[" ); | ||
| console.log("Fq2(\n" + ns(p+n8q*10) + " +\n " + ns(p+n8q*11) +"*u]]" ); | ||
| console.log(s + " [ [Fq2(\n" + ns(p) + " +\n " + ns(p + n8q) + "*u],["); | ||
| console.log("Fq2(\n" + ns(p + n8q * 2) + " +\n " + ns(p + n8q * 3) + "*u],["); | ||
| console.log("Fq2(\n" + ns(p + n8q * 4) + " +\n " + ns(p + n8q * 5) + "*u]]"); | ||
| console.log("[ [Fq2(\n" + ns(p + n8q * 6) + " +\n " + ns(p + n8q * 7) + "*u],["); | ||
| console.log("Fq2(\n" + ns(p + n8q * 8) + " +\n " + ns(p + n8q * 9) + "*u],["); | ||
| console.log("Fq2(\n" + ns(p + n8q * 10) + " +\n " + ns(p + n8q * 11) + "*u]]"); | ||
| } |
There was a problem hiding this comment.
Same as the bn128 thread: no-unused-vars was not running because the flat config's rules: key clobbered ...js.configs.recommended. Fixed the config and re-added the // eslint-disable-next-line no-unused-vars suppressions on these debug helpers.
| import { describe, assert, it, beforeAll } from "vitest"; | ||
|
|
||
| const baseTables = Uint8Array.from(atob(pedersenParamsCode), c => c.charCodeAt(0)); | ||
|
|
There was a problem hiding this comment.
atob is a Node global (Node 16+) and is included in globals.node, so no-undef does not flag it and lint passes. (It also was not running at all due to the config issue noted in the other thread, now fixed.) No change needed.
| "import": "./index.js", | ||
| "require": "./build/main.cjs" | ||
| }, | ||
| "./build/*": "./build/*" |
There was a problem hiding this comment.
Build subpaths are consumed via dynamic import() — e.g. ffjavascript does await import("wasmcurves/build/bn128_wasm_gzip.js") — which works since the build files are ESM. require() of build subpaths is not a path we use. Leaving as-is; if CJS require() of subpaths is ever needed we can add conditional import/require exports plus a .cjs build artifact.
| run: npm ci | ||
|
|
||
| - name: Install Playwright dependencies | ||
| run: npx playwright install |
There was a problem hiding this comment.
Applied → npx playwright install --with-deps chromium.
- Updated eslint.config.js to directly include recommended configurations. - Added eslint-disable comments for unused variables in bls12381.js and bn128.js to prevent linting errors. - Corrected assertion syntax in bls12381.js to use assert.equal for proper comparison.
|
For comparison: I re-applied this migration on top of our optimization stack rather than replaying it, so the MSM/CIOS/memory work is preserved and a couple of bugs are fixed. See #76 (based on our optimization branch, so its diff is the migration alone). Migration-vs-migration diff: feat/esm-migration...feature/esm-tooling Deviations are called out in #76's description. |
module.exportstoexport defaultor named exports in various files including build scripts and utility functions.beforeAllinstead ofbeforefor asynchronous setup.