opentelemetry-js: Named esm import fails on v1.15.0

I would appreciate advice on how to add a test case to ensure that using named exports from a .mjs file works. I don’t know the test suite well.

I was able to repro this issue using the example app I added for esm-http-ts when I installed from the published package (Named export 'ConsoleSpanExporter' not found).

My thought is similar to a point in the comment above - we use this app in a smoke test. I’m not sure if we already do that or something similar elsewhere in this repo, but I can take a look at that.

Similar sudden break for me SyntaxError: Named export 'ConsoleSpanExporter' not found. The requested module '@opentelemetry/sdk-trace-base' is a CommonJS module, which may not support all module.exports as named exports.

While I was writing this, @MSNev you again mentioned export * and I finally connected that you are probably saying what I say in “don’t use export *” below. If so, then this may be the fastest fix. I didn’t initially understand your meaning yesterday in https://github.com/open-telemetry/opentelemetry-js/issues/3976#issuecomment-1633132963

Some possible solutions (I haven’t tried these out).

exports.import + ESM wrapper

A small ESM export wrapper around the CommonJS module could be written, and then use “exports.import” in “package.json” to point to it. This was mentioned at https://github.com/open-telemetry/opentelemetry-js/issues/3989#issuecomment-1634557567

This is basically “Approach # 1” from https://nodejs.org/api/packages.html#writing-dual-packages-while-avoiding-or-minimizing-hazards

I believe tslib itself provides an example of this. The ESM wrapper is here: https://github.com/microsoft/tslib/blob/main/modules/index.js Here is their package.json “exports” definition: https://github.com/microsoft/tslib/blob/2.6.0/package.json#L29-L46

Note that there is a lot going on in that block, much that I don’t grok. The recent tslib git history shows a lot of changes to that block, which may indicate that best practices here are still unclear. Originally tslib’s “export” block was just:

    "exports": {
        ".": {
            "import": "./modules/index.js",
            "default": "./tslib.js"
        },
        "./": "./"
    }

Needing a separate ESM wrapper might be a maintenance burden:

1. for top-level exports, and
2. if additional entry points are defined, as being discussed in https://github.com/open-telemetry/opentelemetry-js/issues/3908

exports.import + fixed build/esm

Add the following, or a variation:

    "exports": {
        "import": "./build/esm/index.js",
        "default": "./build/src/index.js"
    }

and fix “build/esm/…” to work. (Currently it apparently works with Webpack and other bundlers, but the emitted JS import statements don’t include file extensions, which is required. See https://nodejs.org/api/esm.html#mandatory-file-extensions)

This is basically “Approach # 2” from https://nodejs.org/api/packages.html#writing-dual-packages-while-avoiding-or-minimizing-hazards along with the potential pitfall of the package being loaded twice (once as CJS and once as ESM).

don’t use export *

I.e. do this as a starter patch:

diff --git a/packages/opentelemetry-resources/src/index.ts b/packages/opentelemetry-resources/src/index.ts
index 3f66901f..a198a9d0 100644
--- a/packages/opentelemetry-resources/src/index.ts
+++ b/packages/opentelemetry-resources/src/index.ts
@@ -14,8 +14,8 @@
  * limitations under the License.
  */

-export * from './Resource';
-export * from './IResource';
+export { Resource } from './Resource';
+export { IResource } from './IResource';
 export * from './platform';
 export * from './types';
 export * from './config';

I think this means that tslib.__exportStar() won’t get used and we won’t hit the issue. (I haven’t tried this, so I’m not sure if it works.)

I just opened a separate issue for the ESM problems: #3989

I’m rolling back to 1.14.0 for now. This issue also led me to discover that your ESM output is completely broken. First of all, the package.json does not contain "type": "module" so ES Modules will never be loaded from this package. And even if I add this line, the code inside ./build/esm is broken anyway.

For example, @opentelemetry/core/build/esm/index.d.ts has lines such as:

export * from './baggage/propagation/W3CBaggagePropagator';

This will not work in ESM because ESM requires the file extension to be in the import path. The line should be:

export * from './baggage/propagation/W3CBaggagePropagator.js';

I suggest you either fix the ESM output to make it actually functional, or remove it altogether. Because right now it just falls back to CJS. Maybe it works in its current state with some exotic bundlers, but it certainly doesn’t follow the ESM spec.

Thanks @trentm for doing the legwork. It seems to me like the quickest way to get this resolved is to roll back the tslib change #3914 and do a patch release until we can determine if this side-effect can be mitigated or a long term fix is found. I suspect our use of main, module, imports etc in packge.json is to blame here.

Maybe we should consider publishing only an ECMAScript module and removing commonjs entirely from our bundle? Node 12 (our minimum node version) and later supports ES modules.