ESM interoperability question

[oxcafedead]

Hi! I am really not very experienced in JS/TS, especially in its complicated module systems.

However, I'd like to bring one concern here. I have issues in using this library for my ESM JavaScript project. When I try to run unit tests using Mocha, after importing the library I get a runtime error, saying it cannot resolve one of the dependencies:

> npx mocha

Error [ERR_MODULE_NOT_FOUND]: Cannot find module 'xxx\node_modules\@microlabs\otel-cf-workers\dist\sampling' imported from xxx\node_modules\@microlabs\otel-cf-workers\dist\index.js        
    at new NodeError (node:internal/errors:393:5)
    at finalizeResolution (node:internal/modules/esm/resolve:323:11)
    at moduleResolve (node:internal/modules/esm/resolve:916:10)
    at defaultResolve (node:internal/modules/esm/resolve:1124:11)
    at nextResolve (node:internal/modules/esm/loader:163:28)
    at ESMLoader.resolve (node:internal/modules/esm/loader:841:30)
    at ESMLoader.getModuleJob (node:internal/modules/esm/loader:424:18)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:76:40)
    at link (node:internal/modules/esm/module_job:75:36)

The reason is that ./dist has index.js which contains CommonJS-compatible imports (without the .js extention) but it is not compatible with ESM. Most probably, TypeScript does some magic resolving modules w/o extensions. However, if we look at the guide for TypeScript, https://www.typescriptlang.org/docs/handbook/esm-node.html it mentions that either one should use ESM-compatible syntax:

// ./bar.ts
import { helper } from "./foo.js"; // works in ESM & CJS
helper();

or have ESM & CommonJS separated by export index files at package.json:

...
"exports": {
        ".": {
            // Entry-point for `import "my-package"` in ESM
            "import": "./esm/index.js",
            // Entry-point for `require("my-package") in CJS
            "require": "./commonjs/index.cjs",
        },
    },
    // CJS fall-back for older versions of Node.js
    "main": "./commonjs/index.cjs",
...

Now, my question is, is it something which be introduced in this library?

I know that as a workaround, I can use some bundler and run my unit tests after building the code, but I guess my proposal may be a more "right" thing to do.

Apologize in advance, if this proposal contains some obvious errors or misunderstanding from my side.

Thanks for the great library!

[evanderkoogh]

Hey! I am absolutely no wizard in module systems either.

But you are absolutely correct in that I would like this to "just work" for everyone.. so certainly going to have a look to see what the best way forward is.

[evanderkoogh]

Ok.. so I have tried to follow the gist of all the module system wizards and it looks like it works now, but I would love for you to give this a go and check if that is indeed actually the case. It was released in 1.0.0-rc3.

[oxcafedead]

@evanderkoogh Thanks for looking at this issue! Can you please check your index.ts? There are a couple of imports without .js left.

UPD For now, I decided to fix them manually to see what will be the result (sorry I didn't do it earlier) Found couple of other places: sdk.ts (the very bottom):export { waitUntilTrace } from './instrumentation/fetch'; provider.ts: import { AsyncLocalStorageContextManager } from './context'

Then I got another error:

file:///xxx/node_modules/@microlabs/otel-cf-workers/dist/esm/spanprocessor.js:2 
import { stateMachine } from 'ts-checked-fsm';
         ^^^^^^^^^^^^
SyntaxError: Named export 'stateMachine' not found. The requested module 'ts-checked-fsm' is a CommonJS module, which 
may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'ts-checked-fsm';
const { stateMachine } = pkg;

I rewrote the import to

import fsm from 'ts-checked-fsm';
import { getActiveConfig } from './config.js';

const { stateMachine } = fsm;

Then I got:

> npx mocha

ReferenceError: caches is not defined
    at _instrumentGlobalCache (file:///xxx/node_modules/@microlabs/otel-cf-workers/dist/esm/instrumentation/cache.js:67:30)

And this is probably because I try to run tests in a NodeJS runtime where there is no CacheStorage available, because it is not a browser nor a Worker runtime like a Miniflare.

I think I will additionally migrate the tests to wrangler dev to use Miniflare then...

Maybe couple of mentioned imports are worth to fix for consistency, though.

Thanks again.

[jfsiii]

This "Are the types wrong?" tool is really helpful for types and more general package issues.

It found a few issues

You can also npm pack a local install and upload it to iterate on the changes. There's also a CLI

[evanderkoogh]

Right..

So I just retested 1.0.0-rc.7 and it passed all of the things.

Can you let me know if this still happens?