Skip to content

Replace CommonJS require usage with dynamic import() in ESM/TypeScript modules #136

Description

@sourcery-ai

Summary

In the current ESM/TypeScript codebase (project is configured with "type": "module"), there is at least one place where CommonJS require is used for lazy-loading a module (specifically, a DSWebSockerHandler from ./websockets). Using require in an ESM environment can fail at runtime in Node or Bun unless createRequire is explicitly set up, introducing a bug risk.

Problem

  • The project is an ES module project ("type": "module" in package.json).
  • A CommonJS-style require call is used inside an ESM/TypeScript module for lazy loading:
    • It appears to be something like const { DSWebSockerHandler } = require("./websockets") (exact code to be confirmed).
  • In pure ESM contexts, require is not available by default and will throw at runtime unless createRequire is used.
  • This can cause runtime errors when the relevant code path is executed.

Suggested Fix

Replace the usage of require with a dynamic import() to preserve lazy-loading and avoid circular dependencies while remaining compatible with ESM:

// Example pattern
let wsHandler: WebSocketHandler | null = null;

async function getWsHandler() {
  if (!wsHandler) {
    const { DSWebSockerHandler } = await import("./websockets");
    wsHandler = new DSWebSockerHandler();
  }
  return wsHandler;
}

Key points:

  • Use await import("./websockets") (possibly inside an async function or callback) instead of require.
  • Type wsHandler as WebSocketHandler | null (or another appropriate interface/type) instead of typeof import("./websockets").DSWebSockerHandler for clearer and more accurate typing.

Action Items

  1. Locate all occurrences of require inside ESM/TypeScript modules in this repository, especially around the WebSocket handler code.
  2. For each occurrence, migrate to await import()-based dynamic imports.
  3. Adjust typings accordingly:
    • Replace typeof import("./websockets").DSWebSockerHandler with a more direct type such as WebSocketHandler | null (or the actual exported type/interface used in ./websockets).
  4. Add/adjust tests to cover the lazy-loaded path to verify no runtime errors occur in Node/Bun.
  5. Confirm that no circular dependency issues are introduced or reintroduced by the change.

Additional Notes

If there are cases where synchronous loading is required and dynamic import() cannot be used directly, consider refactoring the code paths or, as a last resort, using createRequire from the module package — but this should generally be avoided in a pure ESM codebase when dynamic import will suffice.


I created this issue for @Its4Nik from #132 (comment).

Tips and commands

Getting Help

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions