THRIFT-5811: Add ESM support to nodejs codegen
Client: nodejs
Patch: Cameron Martin <cameronm@graphcore.ai>
This closes #3083
This adds a flag to the JS generator to output ES modules instead of CommonJS. This is only valid when targeting node. A lot of the changes here are to test this.
The `testAll.sh` script now generates an ES module version of the services and types, and tests the client and the server with these. This has a few knock-on effects. Firstly, any module that imports a generated ES module must itself be an ES module, since CommonJS modules cannot import ES modules. ES modules also do not support `NODE_PATH`, so instead the tests directory is converted into a node package with a `file:` dependency on the root thrift package.
diff --git a/lib/nodejs/test/binary.test.js b/lib/nodejs/test/binary.test.js
index 343d39a..1e749d8 100644
--- a/lib/nodejs/test/binary.test.js
+++ b/lib/nodejs/test/binary.test.js
@@ -18,7 +18,7 @@
*/
const test = require("tape");
-const binary = require("thrift/binary");
+const binary = require("thrift/lib/nodejs/lib/thrift/binary");
const cases = {
"Should read signed byte": function (assert) {
diff --git a/lib/nodejs/test/client.js b/lib/nodejs/test/client.mjs
similarity index 90%
rename from lib/nodejs/test/client.js
rename to lib/nodejs/test/client.mjs
index 617039b..6200dc6 100644
--- a/lib/nodejs/test/client.js
+++ b/lib/nodejs/test/client.mjs
@@ -19,17 +19,19 @@
* under the License.
*/
-const assert = require("assert");
-const thrift = require("thrift");
-const helpers = require("./helpers");
+import assert from "assert";
+import thrift from "thrift";
+import helpers from "./helpers.js";
-const ThriftTest = require(`./${helpers.genPath}/ThriftTest`);
-const ThriftTestDriver = require("./test_driver").ThriftTestDriver;
-const ThriftTestDriverPromise =
- require("./test_driver").ThriftTestDriverPromise;
-const SecondService = require(`./${helpers.genPath}/SecondService`);
+const ThriftTest = await import(
+ `./${helpers.genPath}/ThriftTest.${helpers.moduleExt}`
+);
+import { ThriftTestDriver, ThriftTestDriverPromise } from "./test_driver.mjs";
+const SecondService = await import(
+ `./${helpers.genPath}/SecondService.${helpers.moduleExt}`
+);
-const { program } = require("commander");
+import { program } from "commander";
program
.option(
@@ -55,6 +57,7 @@
)
.option("--es6", "Use es6 code")
.option("--es5", "Use es5 code")
+ .option("--esm", "Use es modules")
.parse(process.argv);
const opts = program.opts();
@@ -171,4 +174,4 @@
});
}
-exports.expressoTest = function () {};
+export const expressoTest = function () {};
diff --git a/lib/nodejs/test/helpers.js b/lib/nodejs/test/helpers.js
index 51a0523..1ece2f1 100644
--- a/lib/nodejs/test/helpers.js
+++ b/lib/nodejs/test/helpers.js
@@ -18,7 +18,7 @@
*/
"use strict";
-const thrift = require("../lib/thrift");
+const thrift = require("thrift");
module.exports.transports = {
buffered: thrift.TBufferedTransport,
@@ -32,7 +32,36 @@
header: thrift.THeaderProtocol,
};
-module.exports.ecmaMode = process.argv.includes("--es6") ? "es6" : "es5";
-module.exports.genPath = process.argv.includes("--es6")
- ? "gen-nodejs-es6"
- : "gen-nodejs";
+const variant = (function () {
+ if (process.argv.includes("--es6")) {
+ return "es6";
+ } else if (process.argv.includes("--esm")) {
+ return "esm";
+ } else {
+ return "es5";
+ }
+})();
+
+module.exports.ecmaMode = ["esm", "es6"].includes(variant) ? "es6" : "es5";
+const genPath = (module.exports.genPath = (function () {
+ if (variant == "es5") {
+ return "gen-nodejs";
+ } else {
+ return `gen-nodejs-${variant}`;
+ }
+})());
+
+const moduleExt = (module.exports.moduleExt = variant === "esm" ? "mjs" : "js");
+
+/**
+ * Imports a types module, correctly handling the differences in esm and commonjs
+ */
+module.exports.importTypes = async function (filename) {
+ const typesModule = await import(`./${genPath}/${filename}.${moduleExt}`);
+
+ if (variant === "esm") {
+ return typesModule;
+ } else {
+ return typesModule.default;
+ }
+};
diff --git a/lib/nodejs/test/include.test.mjs b/lib/nodejs/test/include.test.mjs
new file mode 100644
index 0000000..70c7b24
--- /dev/null
+++ b/lib/nodejs/test/include.test.mjs
@@ -0,0 +1,18 @@
+import test from "tape";
+import { IncludeTest as IncludeTestEs5 } from "./gen-nodejs/Include_types.js";
+import { IncludeTest as IncludeTestEs6 } from "./gen-nodejs-es6/Include_types.js";
+import { IncludeTest as IncludeTestEsm } from "./gen-nodejs-esm/Include_types.mjs";
+
+function constructTest(classVariant) {
+ return function (t) {
+ const obj = new classVariant({ bools: { im_true: true, im_false: false } });
+
+ t.assert(obj.bools.im_true === true);
+ t.assert(obj.bools.im_false === false);
+ t.end();
+ };
+}
+
+test("construct es5", constructTest(IncludeTestEs5));
+test("construct es6", constructTest(IncludeTestEs6));
+test("construct esm", constructTest(IncludeTestEsm));
diff --git a/lib/nodejs/test/package-lock.json b/lib/nodejs/test/package-lock.json
new file mode 100644
index 0000000..e7f9543
--- /dev/null
+++ b/lib/nodejs/test/package-lock.json
@@ -0,0 +1,52 @@
+{
+ "name": "test",
+ "lockfileVersion": 3,
+ "requires": true,
+ "packages": {
+ "": {
+ "devDependencies": {
+ "thrift": "file:../../.."
+ }
+ },
+ "../../..": {
+ "version": "0.22.0",
+ "dev": true,
+ "license": "Apache-2.0",
+ "dependencies": {
+ "browser-or-node": "^1.2.1",
+ "isomorphic-ws": "^4.0.1",
+ "node-int64": "^0.4.0",
+ "q": "^1.5.0",
+ "ws": "^5.2.3"
+ },
+ "devDependencies": {
+ "@eslint/js": "^9.18.0",
+ "@types/node": "^22.10.5",
+ "@types/node-int64": "^0.4.29",
+ "@types/q": "^1.5.1",
+ "buffer-equals": "^1.0.4",
+ "commander": "^13.0.0",
+ "connect": "^3.6.6",
+ "eslint": "^9.18.0",
+ "eslint-config-prettier": "^10.0.1",
+ "eslint-plugin-prettier": "^5.2.1",
+ "globals": "^15.14.0",
+ "html-validator-cli": "^2.0.0",
+ "jsdoc": "^4.0.2",
+ "json-int64": "^1.0.2",
+ "nyc": "^15.0.0",
+ "prettier": "^3.4.2",
+ "tape": "^4.9.0",
+ "typescript": "^5.7.2",
+ "utf-8-validate": "^5.0.0"
+ },
+ "engines": {
+ "node": ">= 10.18.0"
+ }
+ },
+ "node_modules/thrift": {
+ "resolved": "../../..",
+ "link": true
+ }
+ }
+}
diff --git a/lib/nodejs/test/package.json b/lib/nodejs/test/package.json
new file mode 100644
index 0000000..22220ec
--- /dev/null
+++ b/lib/nodejs/test/package.json
@@ -0,0 +1,5 @@
+{
+ "devDependencies": {
+ "thrift": "file:../../.."
+ }
+}
diff --git a/lib/nodejs/test/server.js b/lib/nodejs/test/server.mjs
similarity index 84%
rename from lib/nodejs/test/server.js
rename to lib/nodejs/test/server.mjs
index b56bea7..7a3c593 100644
--- a/lib/nodejs/test/server.js
+++ b/lib/nodejs/test/server.mjs
@@ -19,11 +19,11 @@
* under the License.
*/
-const fs = require("fs");
-const path = require("path");
-const thrift = require("../lib/thrift");
-const { program } = require("commander");
-const helpers = require("./helpers");
+import fs from "fs";
+import path from "path";
+import thrift from "thrift";
+import { program } from "commander";
+import helpers from "./helpers.js";
program
.option(
@@ -47,11 +47,16 @@
.option("--callback", "test with callback style functions")
.option("--es6", "Use es6 code")
.option("--es5", "Use es5 code")
+ .option("--esm", "Use es modules")
.parse(process.argv);
-const ThriftTest = require(`./${helpers.genPath}/ThriftTest`);
-const SecondService = require(`./${helpers.genPath}/SecondService`);
-const { ThriftTestHandler } = require("./test_handler");
+const ThriftTest = await import(
+ `./${helpers.genPath}/ThriftTest.${helpers.moduleExt}`
+);
+const SecondService = await import(
+ `./${helpers.genPath}/SecondService.${helpers.moduleExt}`
+);
+import { ThriftTestHandler } from "./test_handler.mjs";
const opts = program.opts();
const port = opts.port;
@@ -114,8 +119,8 @@
type === "websocket"
) {
options.tls = {
- key: fs.readFileSync(path.resolve(__dirname, "server.key")),
- cert: fs.readFileSync(path.resolve(__dirname, "server.crt")),
+ key: fs.readFileSync(path.resolve(import.meta.dirname, "server.key")),
+ cert: fs.readFileSync(path.resolve(import.meta.dirname, "server.crt")),
};
}
}
diff --git a/lib/nodejs/test/test-cases.js b/lib/nodejs/test/test-cases.mjs
similarity index 85%
rename from lib/nodejs/test/test-cases.js
rename to lib/nodejs/test/test-cases.mjs
index 98077f7..543e353 100644
--- a/lib/nodejs/test/test-cases.js
+++ b/lib/nodejs/test/test-cases.mjs
@@ -19,13 +19,14 @@
"use strict";
-const helpers = require("./helpers");
-const ttypes = require(`./${helpers.genPath}/ThriftTest_types`);
-const Int64 = require("node-int64");
+import helpers from "./helpers.js";
+import Int64 from "node-int64";
+
+const ttypes = await helpers.importTypes(`ThriftTest_types`);
//all Languages in UTF-8
/*jshint -W100 */
-const stringTest = (module.exports.stringTest =
+export const stringTest =
"Afrikaans, Alemannisch, Aragonés, العربية, مصرى, " +
"Asturianu, Aymar aru, Azərbaycan, Башҡорт, Boarisch, Žemaitėška, " +
"Беларуская, Беларуская (тарашкевіца), Български, Bamanankan, " +
@@ -50,27 +51,27 @@
"Svenska, Kiswahili, தமிழ், తెలుగు, Тоҷикӣ, ไทย, Türkmençe, Tagalog, " +
"Türkçe, Татарча/Tatarça, Українська, اردو, Tiếng Việt, Volapük, " +
"Walon, Winaray, 吴语, isiXhosa, ייִדיש, Yorùbá, Zeêuws, 中文, " +
- "Bân-lâm-gú, 粵語");
+ "Bân-lâm-gú, 粵語";
/*jshint +W100 */
-const specialCharacters = (module.exports.specialCharacters =
+export const specialCharacters =
'quote: " backslash:' +
" forwardslash-escaped: / " +
" backspace: \b formfeed: \f newline: \n return: \r tab: " +
' now-all-of-them-together: "\\/\b\n\r\t' +
" now-a-bunch-of-junk: !@#$%&()(&%$#{}{}<><><" +
- ' char-to-test-json-parsing: ]] "]] \\" }}}{ [[[ ');
+ ' char-to-test-json-parsing: ]] "]] \\" }}}{ [[[ ';
-const mapTestInput = (module.exports.mapTestInput = {
+export const mapTestInput = {
a: "123",
"a b": "with spaces ",
same: "same",
0: "numeric key",
longValue: stringTest,
stringTest: "long key",
-});
+};
-const simple = [
+export const simple = [
["testVoid", undefined],
["testString", "Test"],
["testString", ""],
@@ -103,32 +104,32 @@
mapout[i] = i - 10;
}
-const deep = [
+export const deep = [
[
"testList",
[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20],
],
];
-const deepUnordered = [
+export const deepUnordered = [
["testMap", mapout],
["testSet", [1, 2, 3]],
["testStringMap", mapTestInput],
];
-const out = new ttypes.Xtruct({
+export const out = new ttypes.Xtruct({
string_thing: "Zero",
byte_thing: 1,
i32_thing: -3,
i64_thing: 1000000,
});
-const out2 = new ttypes.Xtruct2();
+export const out2 = new ttypes.Xtruct2();
out2.byte_thing = 1;
out2.struct_thing = out;
out2.i32_thing = 5;
-const crazy = new ttypes.Insanity({
+export const crazy = new ttypes.Insanity({
userMap: { 5: 5, 8: 8 },
xtructs: [
new ttypes.Xtruct({
@@ -146,7 +147,7 @@
],
});
-const crazy2 = new ttypes.Insanity({
+export const crazy2 = new ttypes.Insanity({
userMap: { 5: 5, 8: 8 },
xtructs: [
{
@@ -164,17 +165,7 @@
],
});
-const insanity = {
+export const insanity = {
1: { 2: crazy, 3: crazy },
2: { 6: { userMap: {}, xtructs: [] } },
};
-
-module.exports.simple = simple;
-module.exports.deep = deep;
-module.exports.deepUnordered = deepUnordered;
-
-module.exports.out = out;
-module.exports.out2 = out2;
-module.exports.crazy = crazy;
-module.exports.crazy2 = crazy2;
-module.exports.insanity = insanity;
diff --git a/lib/nodejs/test/testAll.sh b/lib/nodejs/test/testAll.sh
index 144832e..a3baa6e 100755
--- a/lib/nodejs/test/testAll.sh
+++ b/lib/nodejs/test/testAll.sh
@@ -35,25 +35,23 @@
COUNT=0
-export NODE_PATH="${DIR}:${DIR}/../lib:${NODE_PATH}"
-
testServer()
{
- echo " [ECMA $1] Testing $2 Client/Server with protocol $3 and transport $4 $5";
+ echo " [Variant: $1] Testing $2 Client/Server with protocol $3 and transport $4 $5";
RET=0
if [ -n "${COVER}" ]; then
- ${ISTANBUL} cover ${DIR}/server.js --dir ${REPORT_PREFIX}${COUNT} --handle-sigint -- --type $2 -p $3 -t $4 $5 &
+ ${ISTANBUL} cover ${DIR}/server.mjs --dir ${REPORT_PREFIX}${COUNT} --handle-sigint -- --type $2 -p $3 -t $4 $5 &
COUNT=$((COUNT+1))
else
- node ${DIR}/server.js --${1} --type $2 -p $3 -t $4 $5 &
+ node ${DIR}/server.mjs --${1} --type $2 -p $3 -t $4 $5 &
fi
SERVERPID=$!
sleep 0.1
if [ -n "${COVER}" ]; then
- ${ISTANBUL} cover ${DIR}/client.js --dir ${REPORT_PREFIX}${COUNT} -- --${1} --type $2 -p $3 -t $4 $5 || RET=1
+ ${ISTANBUL} cover ${DIR}/client.mjs --dir ${REPORT_PREFIX}${COUNT} -- --${1} --type $2 -p $3 -t $4 $5 || RET=1
COUNT=$((COUNT+1))
else
- node ${DIR}/client.js --${1} --type $2 -p $3 -t $4 $5 || RET=1
+ node ${DIR}/client.mjs --${1} --type $2 -p $3 -t $4 $5 || RET=1
fi
kill -2 $SERVERPID || RET=1
wait $SERVERPID
@@ -90,10 +88,17 @@
${THRIFT_COMPILER} -o ${DIR} --gen js:node ${THRIFT_FILES_DIR}/v0.16/ThriftTest.thrift
${THRIFT_COMPILER} -o ${DIR} --gen js:node ${THRIFT_FILES_DIR}/JsDeepConstructorTest.thrift
${THRIFT_COMPILER} -o ${DIR} --gen js:node ${THRIFT_FILES_DIR}/Int64Test.thrift
+${THRIFT_COMPILER} -o ${DIR} --gen js:node ${THRIFT_FILES_DIR}/Include.thrift
mkdir ${DIR}/gen-nodejs-es6
${THRIFT_COMPILER} -out ${DIR}/gen-nodejs-es6 --gen js:node,es6 ${THRIFT_FILES_DIR}/v0.16/ThriftTest.thrift
${THRIFT_COMPILER} -out ${DIR}/gen-nodejs-es6 --gen js:node,es6 ${THRIFT_FILES_DIR}/JsDeepConstructorTest.thrift
${THRIFT_COMPILER} -out ${DIR}/gen-nodejs-es6 --gen js:node,es6 ${THRIFT_FILES_DIR}/Int64Test.thrift
+${THRIFT_COMPILER} -out ${DIR}/gen-nodejs-es6 --gen js:node,es6 ${THRIFT_FILES_DIR}/Include.thrift
+mkdir ${DIR}/gen-nodejs-esm
+${THRIFT_COMPILER} -out ${DIR}/gen-nodejs-esm --gen js:node,es6,esm ${THRIFT_FILES_DIR}/v0.16/ThriftTest.thrift
+${THRIFT_COMPILER} -out ${DIR}/gen-nodejs-esm --gen js:node,es6,esm ${THRIFT_FILES_DIR}/JsDeepConstructorTest.thrift
+${THRIFT_COMPILER} -out ${DIR}/gen-nodejs-esm --gen js:node,es6,esm ${THRIFT_FILES_DIR}/Int64Test.thrift
+${THRIFT_COMPILER} -out ${DIR}/gen-nodejs-esm --gen js:node,es6,esm ${THRIFT_FILES_DIR}/Include.thrift
# generate episodic compilation test code
TYPES_PACKAGE=${EPISODIC_DIR}/node_modules/types-package
@@ -121,6 +126,7 @@
node ${DIR}/header.test.js || TESTOK=1
node ${DIR}/int64.test.js || TESTOK=1
node ${DIR}/deep-constructor.test.js || TESTOK=1
+node ${DIR}/include.test.mjs || TESTOK=1
# integration tests
@@ -130,11 +136,11 @@
do
for transport in buffered framed
do
- for ecma_version in es5 es6
+ for gen_variant in es5 es6 esm
do
- testServer $ecma_version $type $protocol $transport || TESTOK=1
- testServer $ecma_version $type $protocol $transport --ssl || TESTOK=1
- testServer $ecma_version $type $protocol $transport --callback || TESTOK=1
+ testServer $gen_variant $type $protocol $transport || TESTOK=1
+ testServer $gen_variant $type $protocol $transport --ssl || TESTOK=1
+ testServer $gen_variant $type $protocol $transport --callback || TESTOK=1
done
done
done
diff --git a/lib/nodejs/test/test_driver.js b/lib/nodejs/test/test_driver.mjs
similarity index 95%
rename from lib/nodejs/test/test_driver.js
rename to lib/nodejs/test/test_driver.mjs
index 0593aea..eca56ba 100644
--- a/lib/nodejs/test/test_driver.js
+++ b/lib/nodejs/test/test_driver.mjs
@@ -26,15 +26,20 @@
// supports an optional callback function which is called with
// a status message when the test is complete.
-const test = require("tape");
+import test from "tape";
-const helpers = require("./helpers");
-const ttypes = require(`./${helpers.genPath}/ThriftTest_types`);
-const TException = require("thrift").Thrift.TException;
-const Int64 = require("node-int64");
-const testCases = require("./test-cases");
+import helpers from "./helpers.js";
+import thrift from "thrift";
+import Int64 from "node-int64";
+import * as testCases from "./test-cases.mjs";
-exports.ThriftTestDriver = function (client, callback) {
+const ttypes = await import(
+ `./${helpers.genPath}/ThriftTest_types.${helpers.moduleExt}`
+);
+
+const TException = thrift.Thrift.TException;
+
+export const ThriftTestDriver = function (client, callback) {
test(
"NodeJS Style Callback Client Tests",
{ skip: helpers.ecmaMode === "es6" },
@@ -162,7 +167,7 @@
}
};
-exports.ThriftTestDriverPromise = function (client, callback) {
+export const ThriftTestDriverPromise = function (client, callback) {
test("Promise Client Tests", function (assert) {
const checkRecursively = makeRecursiveCheck(assert);
diff --git a/lib/nodejs/test/test_handler.js b/lib/nodejs/test/test_handler.mjs
similarity index 95%
rename from lib/nodejs/test/test_handler.js
rename to lib/nodejs/test/test_handler.mjs
index a6a6fc2..a378fe1 100644
--- a/lib/nodejs/test/test_handler.js
+++ b/lib/nodejs/test/test_handler.mjs
@@ -19,9 +19,11 @@
//This is the server side Node test handler for the standard
// Apache Thrift test service.
-const helpers = require("./helpers");
-const ttypes = require(`./${helpers.genPath}/ThriftTest_types`);
-const TException = require("thrift").Thrift.TException;
+import helpers from "./helpers.js";
+import thrift from "thrift";
+
+const ttypes = await helpers.importTypes(`ThriftTest_types`);
+const TException = thrift.Thrift.TException;
function makeSyncHandler() {
return function (thing) {
@@ -217,4 +219,4 @@
asyncHandlers[label] = makeAsyncHandler(label);
});
-exports.ThriftTestHandler = asyncHandlers;
+export { asyncHandlers as ThriftTestHandler };