ESM output not functional

paulius-valiunas commented 1 year ago

What happened?

Steps to Reproduce

import { W3CBaggagePropagator } from "@opentelemetry/core"

Expected Result

being able to compile and run the code

Actual Result

Runtime error:

SyntaxError: Named export 'W3CBaggagePropagator' not found. The requested module '@opentelemetry/core' 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 '@opentelemetry/core';
const { W3CBaggagePropagator } = pkg;

Notice how it says "is a CommonJS module" even though the package contains an ESM directory. I added "type": "module" to node_modules/@opentelemetry/core/package.json and I got a different error when compiling:

error TS2305: Module '"@opentelemetry/core"' has no exported member 'W3CBaggagePropagator'.

Additional Details

The ESM output is broken and doesn't follow ESM spec. 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.

OpenTelemetry Setup Code

No response

package.json

No response

Relevant log output

No response

dyladan commented 1 year ago

The ESM output comes from typescript and works in at least rollup and webpack, and is primarily for web browser bundling. For direct node use we are a commonjs module at least for now. We are willing to revisit that decision if there is interest and benefit.

For now it seems like the issue is actually not that the ESM is broken but that the CommonJS fails to load when loaded from an ESM application. Also a major issue, but a little bit different.

paulius-valiunas commented 1 year ago

I would say the thing you're doing with CJS is more of a workaround than a proper solution. Also, I'm very surprised this works with webpack, because it's not standard ESM and it all looks like one big hack.

These days everyone's migrating to ESM in Node.js so you will have to drop CJS at some point. That would also have the benefit of reducing the size of node_modules, since bundlers are not normally used with Node.js. I just think that fixing what you currently have (it's as easy as adding .js to your imports and "type": "module" to your package.json) would be a good step towards doing that in the future. Instead of making all Node.js projects rely on your CJS output, it would then only be a fallback if the runtime doesn't support ESM.

dyladan commented 1 year ago

I would say the thing you're doing with CJS is more of a workaround than a proper solution.

Can you explain what you mean by this? The module is a regular CommonJS module. We're not working around anything here. Any workarounds or nonstandard practice are related to ESM not CJS.

These days everyone's migrating to ESM in Node.js so you will have to drop CJS at some point.

Maybe true but if we publish as an ES module, we can't be used from CJS projects without code changes. Something we're not willing to do until at least the next major version revision.

That would also have the benefit of reducing the size of node_modules, since bundlers are not normally used with Node.js.

Mostly true but we definitely see users with bundlers in node for one reason or another pretty regularly.

I just think that fixing what you currently have (it's as easy as adding .js to your imports and "type": "module" to your package.json) would be a good step towards doing that in the future. Instead of making all Node.js projects rely on your CJS output, it would then only be a fallback if the runtime doesn't support ESM.

I think we may be able to do something like this:

// package.json
{
  "main": "./build/src/index.js",
  "exports": {
    "import": "./build/esm/index.js",
    "require": "./build/src/index.js",
    "default": "./build/src/index.js",
  },
  "type": "module"
}

// tsconfig.esm.json
{
  "extends": "./tsconfig.base.es5.json",
  "compilerOptions": {
    "module": "ES6",
    "moduleResolution": "node16"
  }
}

trentm commented 1 year ago

I think we may be able to do something like this:

Yup, that is a suggestion from https://nodejs.org/api/packages.html#writing-dual-packages-while-avoiding-or-minimizing-hazards Note the differences in "Approach # 1" and "Approach # 2" discussed there.

We'd want to consider whether we want {"exports": { "import": ... to point to a small ESM wrapper -- what the Node.js docs call "Approach # 1". Otherwise, it would be possible to have separate copies of the library loaded (one from CJS, one from ESM). Perhaps the separately loaded copies wouldn't be a concern (instanceof checks failing, separate state).

klippx commented 1 year ago

I agree with @paulius-valiunas this isn't working and I came to this repo only to say the same thing.

What we want to write:

import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions';

What we have to write to make it work:

import * as SemanticConventionsModule from '@opentelemetry/semantic-conventions';
// Do not ask me what they are doing here
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const HackedSemanticConventionsModule = (SemanticConventionsModule as any)
  .default as typeof SemanticConventionsModule;
const { SemanticResourceAttributes } = HackedSemanticConventionsModule;

paulius-valiunas commented 1 year ago

Can you explain what you mean by this? The module is a regular CommonJS module. We're not working around anything here. Any workarounds or nonstandard practice are related to ESM not CJS.

I mean the package is not a pure CJS package, you do have ESM modules in this package but they can't be used. If they can't be used, then why do you have them there in the first place? It doubles the package size for no reason.

Maybe true but if we publish as an ES module, we can't be used from CJS projects without code changes. Something we're not willing to do until at least the next major version revision.

It can. I'm not asking you to remove the CJS modules, they can continue working the exact same way as they are now. If someone uses require with this package, it will ignore the ESM directory the same way it does now, nothing will change in that regard. All I'm asking for is a working import("@opentelemetry/core") without relying on Node.js' backwards compatibility layer. I know it would be a far fetch if you only currently had CJS modules in here, but you already have a hybrid CJS/ESM package, it just needs a little touch up to work properly with all runtimes and bundlers.

Mostly true but we definitely see users with bundlers in node for one reason or another pretty regularly.

Well I didn't say bundlers are never used in Node.js, it's just not the most standard approach. I was trying to say the majority of Node.js users don't use bundlers. And that means removing CJS would benefit most (but not all) Node.js users.

I think we may be able to do something like this:

// package.json
{
  "main": "./build/src/index.js",
  "exports": {
    "import": "./build/esm/index.js",
    "require": "./build/src/index.js",
    "default": "./build/src/index.js",
  },
  "type": "module"
}

// tsconfig.esm.json
{
  "extends": "./tsconfig.base.es5.json",
  "compilerOptions": {
    "module": "ES6",
    "moduleResolution": "node16"
  }
}

There are many ways to do this, I think your example is fine, although it has a quirky side effect of not allowing people to require a specific module (require("@opentelemetry/core/build/src/something")) - because that module will be interpreted as CJS. The solution would be to add a ./build/src/package.json that contained only a "type": "commonjs" line. There are more detailed solutions here: https://2ality.com/2019/10/hybrid-npm-packages.html

paulius-valiunas commented 1 year ago

I think we may be able to do something like this:

Yup, that is a suggestion from nodejs.org/api/packages.html#writing-dual-packages-while-avoiding-or-minimizing-hazards Note the differences in "Approach # 1" and "Approach # 2" discussed there.

We'd want to consider whether we want {"exports": { "import": ... to point to a small ESM wrapper -- what the Node.js docs call "Approach # 1". Otherwise, it would be possible to have separate copies of the library loaded (one from CJS, one from ESM). Perhaps the separately loaded copies wouldn't be a concern (instanceof checks failing, separate state).

That's a very good point, I didn't initially think about all the global/static state you have in these packages. For example, different modules call trace.getActiveSpan(), but they get different instances of trace - will they get the same span? Will one of them get an empty span or no span at all?... I'm not very well versed in the architecture of these packages but it sounds like it could potentially be a problem. But then I guess that's the same as people trying to use different versions of opentelemetry in their packages (does that currently work? I haven't tested...)

luxaritas commented 1 year ago

Something to add here. I'm attempting to use vite-node in development, and wind up running into the following:

 /app/node_modules/@opentelemetry/resources/build/esm/platform/node/machine-id/getMachineId-linux.js:1
 import { __awaiter, __generator, __values } from "tslib";
 ^^^^^^

 SyntaxError: Cannot use import statement outside a module
     at internalCompileFunction (node:internal/vm:73:18)
     at wrapSafe (node:internal/modules/cjs/loader:1176:20)
     at Module._compile (node:internal/modules/cjs/loader:1218:27)
     at Object.Module._extensions..js (node:internal/modules/cjs/loader:1308:10)
     at Module.load (node:internal/modules/cjs/loader:1117:32)
     at Function.Module._load (node:internal/modules/cjs/loader:958:12)
     at Module.require (node:internal/modules/cjs/loader:1141:19)
     at require (node:internal/modules/cjs/helpers:110:18)
    at /app/node_modules/@opentelemetry/resources/src/platform/node/machine-id/getMachineId.ts:25:25
     at ViteNodeRunner.directRequest (file:///app/node_modules/vite-node/dist/client.mjs:341:5)

Node.js v18.16.0

If I revert to before the tslib change

 /app/node_modules/@opentelemetry/resources/build/esm/platform/node/machine-id/getMachineId-linux.js:63
 import { promises as fs } from 'fs';
 ^^^^^^

 SyntaxError: Cannot use import statement outside a module
     at internalCompileFunction (node:internal/vm:73:18)
     at wrapSafe (node:internal/modules/cjs/loader:1176:20)
     at Module._compile (node:internal/modules/cjs/loader:1218:27)
     at Object.Module._extensions..js (node:internal/modules/cjs/loader:1308:10)
     at Module.load (node:internal/modules/cjs/loader:1117:32)
     at Function.Module._load (node:internal/modules/cjs/loader:958:12)
     at Module.require (node:internal/modules/cjs/loader:1141:19)
     at require (node:internal/modules/cjs/helpers:110:18)
     at /app/node_modules/@opentelemetry/resources/src/platform/node/machine-id/getMachineId.ts:25:25
    at ViteNodeRunner.directRequest (file:///app/node_modules/vite-node/dist/client.mjs:341:5)

Node.js v18.16.0

It seems like there's a good chance this is caused by node_modules/@opentelemetry/resources/build/esm/platform/node/machine-id/getMachineId.js using require to import that file which uses import. import()ing a CJS file is fine, but going the other way around causes issues. :)

paulius-valiunas commented 1 year ago

@luxaritas you're seeing this error because you're using the ESM output but there's no "type": "module" in its package.json. It doesn't matter if you use require or import to import the getMachineId-linux.js module, this will always happen because the module itself is broken. This is exactly the problem I was talking about: the ESM output of these packages is incomplete and cannot be used in Node.js.

luxaritas commented 1 year ago

NB: I did try adding it, but it complained about (IIRC) the module actually being CJS and not ESM. Either way, it is also broken in this second way

paulius-valiunas commented 1 year ago

NB: I did try adding it, but it complained about (IIRC) the module actually being CJS and not ESM. Either way, it is also broken in this second way

It must have been something else then, because this field clearly instructs Node.js to treat all .js files as ESM. (unless you require them)

edit: now that I think of it, I guess you might be right, requiring that module might have forced it to be loaded as CJS. I'll need to do some experiments, this looks like an interesting case

chronicIntrovert commented 1 year ago

Currently getting

SyntaxError: Cannot use import statement outside a module ❯ node_modules/@opentelemetry/resources/src/platform/node/machine-id/getMachineId.ts:23:11

when trying to run vitest in both Github action CI and locally. Is there a workaround for this right now?

acidoxee commented 1 year ago

It must have been something else then, because this field clearly instructs Node.js to treat all .js files as ESM. (unless you require them)

To me all files in the build/esm folder are invalid, since none of their imports contain a file extension. That's the most obvious problem, but there might be more. Simply adding a package.json file in build/esm and putting { "type": "module" } in it isn't enough. Everything in there needs to be pure and valid ESM, which it currently isn't, so I agree that the ESM output of this package is not functional.

Regarding my usecase, we're currently writing ESM-only code for Node, which is bundled along with its dependencies with CDK (which uses esbuild). This code is deployed on several lambdas (there are loads of entrypoints, one for each lambda). We're using OpenTelemetry as a direct dependency, primarily for tracing purposes, across all of our packages.

Since a small bundle size is paramount to achieve decent cold starts on AWS Lambda, we've started inspecting bundles to find bottlenecks. What we've found is that OpenTelemetry is consistently in the top 5 heaviest dependencies in there, with just @opentelemetry/core, @opentelemetry/api and @opentelemetry/semantic-conventions taking around 65kb, which is huge compared to our total bundles.

What was surprising though is that those modules are loaded from the CJS output, not the ESM one. My guess would be that esbuild detects bad ESM files/config and defaults to the CJS output, but I'm not entirely sure about that process. Using TypeScript's go-to-source on our OpenTelemetry libs' imports also brings us to the CJS output, not the ESM one, so TS also seems to pick up on that.

I believe going the ESM-only path would be the easiest solution for OpenTelemetry maintainers in the long run, and it would definitely be the safest bet regarding tree-shaking capabilities. I reckon dual exports of both CJS and ESM can be tricky, since you either deteriorate tree-shaking by reusing CJS outputs from ESM, or you risk having two separate instances between CJS and ESM-using code. If OpenTelemetry really had to support both module systems in the future, I'd personally really appreciate having a "real", dedicated ESM entrypoint that's pure ESM, not just an ESM wrapper around a CJS entrypoint.

mikearnaldi commented 12 months ago

I also confirm that the modules are invalid esm, as https://cdn.jsdelivr.net/npm/@opentelemetry/exporter-metrics-otlp-proto@0.45.1/build/esnext/OTLPMetricExporter.js shows there is lack of '.js' extension in imports and this breaks usage. We are currently forcing usage of CJS imports which isn't the best.

After a lot of attempts in Effect we decided to not follow the re-export of cjs from esm at all as it breaks compatibility with lots of libraries in the ecosystem, we ended up with isolating global state inside an object embedded in globalThis so that different module instances can access the same state, if you're curious about the discussion you can find it in: https://github.com/Effect-TS/effect/issues/1561

nitedani commented 12 months ago

https://publint.dev/@opentelemetry/resources@1.18.1 https://publint.dev/@opentelemetry/instrumentation@0.45.1 https://publint.dev/@opentelemetry/api@1.7.0

This tool detects the issues. I checked a few packages, and they contain a lot of errors.

klippx commented 12 months ago

@dyladan Sorry to ping you, but you self-assigned this and marked it as P1. Did you by any chance find some time to spend on this? 🙏

dyladan commented 12 months ago

@klippx I have not unfortunately

sawmurai commented 11 months ago

Quoting from the original issue description:

Maybe it works in its current state with some exotic bundlers, but it certainly doesn't follow the ESM spec.

With vite 5 it no longer works at all as the missing .js file extensions in the ESM build cause errors.

RichiCoder1 commented 11 months ago

This should be a relatively easy matter of setting exports correctly. I think I could help contribute this, only caveat might be bundling/types.

RichiCoder1 commented 11 months ago

In theory removing module, esnext, and browser, and doing the below would resolve it, though I think there's still a posibility of ambiguity due to some tools requiring explicit extensions (e.g. .mjs):

  "exports": {
    ".": {
      "browser": {
        "import": {
          "types": "./build/esm/platform/browser/index.d.ts",
          "default": "./build/esm/platform/browser/index.js"
        }
      },
      "node": {
        "import": {
          "types": "./build/esm/index.d.ts",
          "default": "./build/esm/index.js"
        },
        "require": {
          "types": "./build/src/index.d.ts",
          "default": "./build/src/index.js"
        }
      },
      "import": {
        "types": "./build/esm/index.d.ts",
        "default": "./build/esm/index.js"
      },
      "require": {
        "types": "./build/src/index.d.ts",
        "default": "./build/src/index.js"
      }
    }
  },

The above is also technically a breaking change since it obscures package internals, but that's a simple fix (add a star condition).

Edit: This also runs into the issue pointed out previously in the thread where this breaks implicit state between different imports. An end-user application would need to uniformly reference an export or implicit state would break.

klippx commented 11 months ago

If I read the situation correctly @dyladan isn't too interested in dealing with ESM aspect of opentelemetry, hopefully he can 👍 that you take a stab at it and help get it done.

The fix SHOULD be simple, but it might not AS simple as you think.

Right now if we do this: import opentelemetry from '@opentelemetry/api'; in an ESM project we get the CJS code. The ESM code is dead weight, you have to deeply import (import opentelemetry from '@opentelemetry/api/build/esm/index.js';) to get it and no one does that. No one. Because even if they did, it wouldn't work because they would get:

Warning: To load an ES module, set "type": "module" in the package.json or use the .mjs extension.
error: Unexpected token 'export'

Since the ESM built artefact is not functional (I guess that's how the title was chosen for this issue).

So it's not as simple as fixing a few exports, you also need to ensure valid ESM output (.js file endings) and for that you need to roll up your sleeves and get in to the nitty gritty of how the project build pipeline for this monorepo.

RichiCoder1 commented 11 months ago

So it's not as simple as fixing a few exports, you also need to ensure valid ESM output (.js file endings) and for that you need to roll up your sleeves and get in to the nitty gritty of how the project build pipeline for this monorepo.

Yah, I think this is the crux of the issue (aside from the shared state issue). TypeScript doesn't natively offer the tools today to support dual packaging. So we'd need to introduce another bundler/packager to at least handle emitting correctly postfixed files.

paulius-valiunas commented 11 months ago

So it's not as simple as fixing a few exports, you also need to ensure valid ESM output (.js file endings) and for that you need to roll up your sleeves and get in to the nitty gritty of how the project build pipeline for this monorepo.

Yah, I think this is the crux of the issue (aside from the shared state issue). TypeScript doesn't natively offer the tools today to support dual packaging. So we'd need to introduce another bundler/packager to at least handle emitting correctly postfixed files.

Typescript is absolutely capable of outputting valid ESM code, you don't need a bundler for that. You just need to build it with a different tsconfig file. So if you want both ESM and CJS, you just run tsc twice with different configs.

luxaritas commented 11 months ago

Typescript is absolutely capable of outputting valid ESM code, you don't need a bundler for that. You just need to build it with a different tsconfig file. So if you want both ESM and CJS, you just run tsc twice with different configs.

That is exactly what is being done in this repo. I did some additional digging, and the problem with getMachineId in @opentelemetry/resources is that it uses synchronous require to pull in the code for the correct platform, which ts is unable to translate to something reasonable.

paulius-valiunas commented 11 months ago

Typescript is absolutely capable of outputting valid ESM code, you don't need a bundler for that. You just need to build it with a different tsconfig file. So if you want both ESM and CJS, you just run tsc twice with different configs.

That is exactly what is being done in this repo. I did some additional digging, and the problem with getMachineId in @opentelemetry/resources is that it uses synchronous require to pull in the code for the correct platform, which ts is unable to translate to something reasonable.

The main problem is that there are no .js extensions in import statements in the source code (Typescript doesn't add them), and the type property is missing from the package.json. There may be some other problems, like the getMachineId function you mentioned - but none of them should require a separate bundler to fix.

dyladan commented 11 months ago

If I read the situation correctly @dyladan isn't too interested in dealing with ESM aspect of opentelemetry, hopefully he can 👍 that you take a stab at it and help get it done.

I wouldn't say I'm not interested, but I have limited time and this hasn't bubbled up the priority queue for me yet. I would indeed be happy for someone to contribute here.

The fix SHOULD be simple, but it might not AS simple as you think.

+1 to this. In my experience, double-publishing ESM and commonjs is never as simple as it should be. I'm happy to be proven wrong with a PR in this regard.

JamieDanielson commented 11 months ago

While looking at this issue on nodejs I came across a link to this PR on aws-powertools repo that I think we can use as a guide. The PR includes an excellent description with references to various sources and reasonings for changes. There are then multiple follow-up PRs for other packages there, which is probably what we'd need to do as well because there are too many files to include in one PR.

For each package we'll need to:

Change the package type to module as default
Add dual exports to package.json
Add/update tsconfig.esm.json
Have separate build scripts for cjs and esm, along with command line addition of type:module and type:commonjs in a package.json, which allows us to keep the .js suffix
Fix every relative import to be ./fileName.js

I took a pass at this on the Express instrumentation package because it's much smaller in scope. One thing I'm not sure of is where esnext comes into play here. From other testing, some build tools seem to need it (like vite), but most things I've seen only show CJS and ESM, with ESM actually using ESNext 🤔

JakeGinnivan commented 11 months ago

https://www.typescriptlang.org/docs/handbook/modules/reference.html#node16-nodenext is worth a read.

If you use nodenext in the esm type checking it will enforce file extensions etc in the type checker which could help prevent downstream issues.

klippx commented 10 months ago

I think @JamieDanielson summarized pretty accurately what needs to be done and as @JakeGinnivan points out there are some nice things to add to tsconfig.esm.json.

To get even more practical, and to address the concerns of @dyladan, I can share some experience in migrating a package that used to be published to npm only as CJS, to instead get published as both ESM and CJS: To ensure both that the old CJS users are not broken AND ensuring it works as expected for the new target audience (the ESM users) I would recommend a "smoke-test" strategy that I employed which was to set up a monorepo whose only point is to ensure imports are working correctly for different consumer types - from types, to tests, to runtime. And different consumer types being e.g.

consumer package.json is using type: commonjs or type: module
consumer is using const x = require y
consumer is using import x = require y
consumer is using import x from y
consumer is using deep imports or not, (e.g import x from 'lib/some/deep/path') - this is especially important if READMEs/docs mention this a lot and it wont work for ESM if you dont expose it explicitly in package.json

All in all a smoke test like this ensures that the types compile, that the tests runs, and finally also that runtime (integration) works for different projects. These are things that can all individually break. This is not an exhaustive test in any sense, it just smoke test to see that we are in the ball park of NOT fundamentally and utterly breaking anyone.

The smoke-test consumer project is here https://github.com/klippx/mappersmith-consumer if you want some inspiration, it is not immediately reusable for this project, but can serve as inspo / fast track to get something similar up.

jrestall commented 10 months ago

Any workarounds for this issue in the meantime? I'm also getting errors when using opentelemetry with vite-node. @luxaritas did you resolve by pinning to an old version?

/node_modules/@opentelemetry/resources/build/esm/platform/node/machine-id/getMachineId-darwin.js:52
import { execAsync } from './execAsync';
^^^^^^
SyntaxError: Cannot use import statement outside a module

luxaritas commented 10 months ago

@jrestall I'm currently getting it to work by using patch-package to remove the module and esnext keys from the package.json, letting it fall back to CJS.

shinebayar-g commented 10 months ago

Excuse me, is @opentelemetry packages usable in ESM projects? I have "type": "module" in my package.json and I can't seem to get it working nor I could find any example. (Obviously not NodeJS expert)

2013xile commented 10 months ago

Currently getting

SyntaxError: Cannot use import statement outside a module ❯ node_modules/@opentelemetry/resources/src/platform/node/machine-id/getMachineId.ts:23:11

when trying to run vitest in both Github action CI and locally. Is there a workaround for this right now?

Same. I add resolve alias in vitest config for temporarily fixing it.

{
    find: '@opentelemetry/resources',
    replacement: 'node_modules/@opentelemetry/resources/build/src/index.js',
}

JamieDanielson commented 10 months ago

https://www.typescriptlang.org/docs/handbook/modules/reference.html#node16-nodenext is worth a read.

If you use nodenext in the esm type checking it will enforce file extensions etc in the type checker which could help prevent downstream issues.

@JakeGinnivan Thanks for the link! This was also suggested in #4396 but unfortunately nodenext isn't available as a module option until TypeScript v4.7, and there are breaking changes in TypeScript from the current version 4.4.4 to 4.7 (see also this comment on #3952).

There is a plan to update to TypeScript 5 in the next major SDK version 🤔. I think using nodenext would be the best approach after upgrading TypeScript, but in the meantime I guess we can still do the separate tsconfig files.

@klippx Thanks for confirmation and also for the link to the smoke test project! That looks like an interesting approach worth exploring, and very different from what I originally had in mind when I created this issue to add smoke tests. I'll check out both ideas and see if I can put something together to help improve confidence as we keep going down the path of trying to make ESM fully functional in all expected scenarios.

RichiCoder1 commented 10 months ago

Yah, I think this might end up be a major-version kinda fix. Possibly three things need to happen:

Either update libraries to store vital state in global via symbols or add some docs about making sure a project is using the same version and import (esm vs. cjs) to prevent state issues with global tracing.
Use package.json export conditions to ensure each runtime is getting the correct build. (This would also make it easier to vendor specific build files for platforms that use package conditions like workers or react-native). Use a tool like publint and/or arethetypeswrong to validate the export conditions and types are processing correctly in addition to integration smoke tests.
I'd also recommend looking into a tool like tsup or another higher level tool than ts's own compiler which operates at a higher level, handles things like type bundling, and emits the correct extensions for each module type.

dyladan commented 9 months ago

@RichiCoder1 luckily we are already working on the next major version in the next branch

RichiCoder1 commented 9 months ago

@RichiCoder1 luckily we are already working on the next major version in the next branch

🥳 awesome! I don't know how best I can help, but happy to help!

dpnova commented 3 months ago

@jrestall I'm currently getting it to work by using patch-package to remove the module and esnext keys from the package.json, letting it fall back to CJS.

FTR this worked with pnpm patch too.

open-telemetry / opentelemetry-js