Error importing trino-client because of type mismatch

[ZeRego]

The trino-client configuration is inconsistent in 0.2.4, and this mismatch is causing an error.

const trino_client_1 = require("trino-client");
                       ^

Error [ERR_REQUIRE_ESM]: require() of ES Module .../Documents/lightdash/node_modules/trino-client/dist/index.js from .../Documents/lightdash/packages/warehouses/dist/warehouseClients/TrinoWarehouseClient.js not supported.
index.js is treated as an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which declares all .js files in that package scope as ES modules.
Instead either rename index.js to end in .cjs, change the requiring code to use dynamic import() which is available in all CommonJS modules, or change "type": "module" to "type": "commonjs" in .../Documents/lightdash/node_modules/trino-client/package.json to treat all .js files as CommonJS (using .mjs for all ES modules instead).

Here's why:

type: "module" in package.json:
This tells Node.js to treat all .js files in the package as ES Modules by default. This affects how Node.js resolves and executes the files.

"module": "commonjs" in tsconfig.json:
This tells TypeScript to compile the code as CommonJS modules. The emitted files will use CommonJS syntax (e.g., require instead of import).

Why is This Problematic?

When type: "module" is set:

Node.js expects the files to use ES Module syntax (import/export).
But the compiled CommonJS code (require/module.exports) doesn't match this expectation.
This creates conflicts because Node.js cannot correctly interpret CommonJS output in a package marked as an ES Module.

Solution

I don't know why type: "module" was recently added to package.json but the simplest solution is to rollback this change.
Note that 0.2.3 works fine but we would like to use the latest version to patch a security vulnerability in axious which is a dependency from trino-client.

[nineinchnick]

We should revert this change, it was added in #660 because the ESLint update tool by default converts the config into ECMAScript, instead of JavaScript. OTOH, I see #720 is moving closer to ESModules though, but not sure when it'll be ready.

[mosabua]

Sounds good .. Want to send a PR to remove that and cut a new release @nineinchnick or @ZeRego .. or will your PR land soon @regadas ?

[mosabua]

Ultimately we want users to be able to use the client in as many JS versions/stacks as possible.

[regadas]

Yeah I was reaching this conclusion over the weekend; I'll revert the change and release it today. Regarding the draft PR, it shouldn't impact things as it only tackles eslint and jest.

[nineinchnick]

The default eslint config after update looks like this: https://github.com/trinodb/github-actions/blob/main/action-surefire-report/eslint.config.cjs

[mosabua]

Awesome

[mosabua]

Released 0.2.5 .. please confirm that you are all good now

[kylejb]

@mosabua, I updated my trino-client dependency to 0.2.5 as part of my NestJS project and am still seeing the reported error.

I think we need to fix the "main" entry in package.json. That might be all it takes. I'll create a PR for that.