Skip to content

fix!: Convert project to output ESM #110

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.

Sign up for GitHub

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

Closed
gregersrygg wants to merge 1 commit into from
Closed

fix!: Convert project to output ESM #110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions jest.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type { Config } from "jest";

const config: Config = {
preset: "ts-jest/presets/default-esm",
resolver: "ts-jest-resolver",
resetMocks: true,
restoreMocks: true,
transform: {}
Expand Down
80 changes: 20 additions & 60 deletions package-lock.json
View file
Open in desktop

Large diffs are not rendered by default.

7 changes: 4 additions & 3 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"url": "git+https://github.com/microcks/microcks-testcontainers-node.git"
},
"license": "Apache-2.0",
"main": "build/index.js",
"exports": "./build/index.js",
Comment thread
lbroudoux marked this conversation as resolved.
"publishConfig": {
"access": "public"
},
Expand All @@ -23,7 +23,8 @@
"test": "cross-env DEBUG=testcontainers* NODE_OPTIONS='--experimental-vm-modules' jest"
},
"dependencies": {
"testcontainers": "10.16.0"
"testcontainers": "10.16.0",
"ts-jest-resolver": "^2.0.1"

@lbroudoux lbroudoux Jun 5, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't this one be in devDependencies?

},
"devDependencies": {
"@aws-sdk/client-sqs": "3.398.0",
Expand All @@ -46,6 +47,6 @@
"build"
],
"engines": {
"node": ">= 16"
"node": ">= 20"
Comment thread
lbroudoux marked this conversation as resolved.
}
}
6 changes: 3 additions & 3 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
export * from "./microcks-container";
export * from "./microcks-containers-ensemble";
export * from "./microcks-async-minion-container";
export * from "./microcks-container.js";
export * from "./microcks-containers-ensemble.js";
export * from "./microcks-async-minion-container.js";
Comment on lines +16 to +18

@lbroudoux lbroudoux Mar 31, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So we have to reference the *.js files now?

@gregersrygg gregersrygg Apr 1, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes, even though it's typescript:

You must use a .js extension in relative imports even though you're importing .ts files

2 changes: 1 addition & 1 deletion src/microcks-async-minion-container.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

import { AbstractStartedContainer, GenericContainer, StartedNetwork, StartedTestContainer, Wait, getContainerRuntimeClient } from "testcontainers";
import { MicrocksContainer } from "./microcks-container";
import { MicrocksContainer } from "./microcks-container.js";

export class MicrocksAsyncMinionContainer extends GenericContainer {
static readonly MICROCKS_ASYNC_MINION_HTTP_PORT = 8081;
Expand Down
4 changes: 2 additions & 2 deletions src/microcks-containers-ensemble.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ import { CreateQueueCommand, ListQueuesCommand, ReceiveMessageCommand, SQSClient
import { GenericContainer, Network, Wait } from "testcontainers";
import { LocalstackContainer } from "@testcontainers/localstack";
import { RabbitMQContainer } from "@testcontainers/rabbitmq";
import { MicrocksContainersEnsemble } from "./microcks-containers-ensemble";
import { TestRequest, TestResult, TestRunnerType } from "./microcks-container";
import { MicrocksContainersEnsemble } from "./microcks-containers-ensemble.js";
import { TestRequest, TestResult, TestRunnerType } from "./microcks-container.js";
import { WebSocket } from "ws";
import mqtt from "mqtt";
import amqp from "amqplib";
Expand Down
10 changes: 5 additions & 5 deletions src/microcks-containers-ensemble.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@
* limitations under the License.
*/
import { GenericContainer, StartedNetwork, StartedTestContainer, StopOptions, Wait } from "testcontainers";
import { MicrocksContainer, Secret, StartedMicrocksContainer } from "./microcks-container";
import {
AmazonServiceConnection, GenericConnection, KafkaConnection,
MicrocksAsyncMinionContainer, StartedMicrocksAsyncMinionContainer
} from "./microcks-async-minion-container";
import { MicrocksContainer, Secret, StartedMicrocksContainer } from "./microcks-container.js";
import {
AmazonServiceConnection, GenericConnection, KafkaConnection,
MicrocksAsyncMinionContainer, StartedMicrocksAsyncMinionContainer
} from "./microcks-async-minion-container.js";

export class MicrocksContainersEnsemble {
static readonly MICROCKS_CONTAINER_ALIAS = "microcks";
Expand Down
4 changes: 2 additions & 2 deletions tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
"compilerOptions": {
"incremental": true,
"target": "es2022",
"module": "commonjs",
"module": "nodenext",
"declaration": true,
"sourceMap": true,
"strict": true,
"moduleResolution": "node",
"moduleResolution": "nodenext",
Comment on lines +5 to +9

@lbroudoux lbroudoux Mar 31, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These are actually recommendations from testcontainers (see https://github.com/testcontainers/testcontainers-node/blob/main/tsconfig.base.json). What is the impact of changing these?

@gregersrygg gregersrygg Apr 1, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

node10: Formerly known as node, this is the unfortunate default when module is set to commonjs. It’s a pretty good model of Node.js versions older than v12, and sometimes it’s a passable approximation of how most bundlers do module resolution. It supports looking up packages from node_modules, loading directory index.js files, and omitting .js extensions in relative module specifiers. Because Node.js v12 introduced different module resolution rules for ES modules, though, it’s a very bad model of modern versions of Node.js. It should not be used for new projects.
node16: This is the counterpart of --module node16 and --module node18 and is set by default with that module setting. Node.js v12 and later support both ESM and CJS, each of which uses its own module resolution algorithm. In Node.js, module specifiers in import statements and dynamic import() calls are not allowed to omit file extensions or /index.js suffixes, while module specifiers in require calls are. This module resolution mode understands and enforces this restriction where necessary, as determined by the module format detection rules instated by --module node16/node18. (For node16 and nodenext, module and moduleResolution go hand-in-hand: setting one to node16 or nodenext while setting the other to something else is an error.)
nodenext: Currently identical to node16, this is the counterpart of --module nodenext and is set by default with that module setting. It’s intended to be a forward-looking mode that will support new Node.js module resolution features as they’re added.
https://www.typescriptlang.org/docs/handbook/modules/theory.html#module-resolution

When looking at the testcontainers project, I'm afraid they're having the same issue. Also looks like they attempted to convert to ESM a while back, but closed the PR without any reasoning: testcontainers/testcontainers-node#406

I don't have much time to look into this, and it's not a critical issue. We can just continue to use an old version, but it would be nice for others if you either revert the module change in package.json or continue with the change to a proper ESM module. The module type in package.json is looked up at runtime, and type "module" tells the JS host (Node.js) that the javascript in this project is using import/export and not require. See more about how it works here:
https://www.typescriptlang.org/docs/handbook/modules/theory.html#module-resolution

"esModuleInterop": true,
"skipLibCheck": true,
"forceConsistentCasingInFileNames": true,
Expand Down