dd-trace-js
dd-trace-js copied to clipboard
Incorrect exports for TypeScript in ESM mode
Expected behaviour
import tracer from "dd-trace"; works in TypeScript ESM mode
Actual behaviour
Instead of tracer referring to the tracer, an extra .default access is required.
Steps to reproduce See https://arethetypeswrong.github.io/?p=dd-trace%404.2.0 for a description of the issue and suggested fix. A TS project with moduleResolution set to "NodeNext" and package.json's "type" set to "module" reproduces it.
Environment
- Operation system: macOS 13.4
- Node.js version: node 18.x
- Tracer version: 4.2.0
- Agent version: –
- Relevant library versions: –
@benasher44 would you be willing to make a pull request for this?
Closing due to inactivity, if you are still having this problem please feel free to reopen
This is still an issue. I would consider making a PR, but it doesn't look like I can reopen this issue.
@khanayan123 can you please reopen? This is still an issue. The TypeScript ESM support is still incorrect.
Hi @andrewbranch! I'm not sure what to recommend here. The tracer export has the requisite default property on it, so it should work to export = tracer, but I'm not sure how to represent the rest of named exports alongside the default export. I looked at the new module resolution guides published in 5.3, but I haven't been able to figure it out. Any tips would be appreciated!
arethetypeswrong isn't able to find that default property assignment, so the docs it links too aren't the most helpful. The actual problem is this one, which explains how to combine named exports of types with an export =: https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/MissingExportEquals.md
Let me know if that provides enough to get you started.
Ah, thanks @andrewbranch!
@khanayan123 is that enough info for your team to fix? (started working on it myself, but it turned out to be more significant than I think it makes sense for an outside contributor)
@khanayan123 I took a crack at it in #3937, but there's one outstanding issue. @andrewbranch I have the namespace merging set up, but when I attempt to use it in code, it doesn't seem to resolve the tracer/default export as Tracer the interface (resolves to Tracer to the namespace — maybe this isn't right though and I'm misunderstanding the root issue). For example, I can't call tracer.init
It’s a little hard to tell in a big diff for a library I’m not familiar with, but it looks like you have something like this:
// namespace is also a value because it exports a value
declare namespace Tracer {
// export some types
export interface Foo {}
// export a value
export const tracer: Tracer;
export { tracer as default };
}
// namespace/value merges with this type
interface Tracer {}
// export type, value, namespace merge
export = Tracer;
Notice that the export = is not a value tracer: Tracer. It is a value more like { tracer: Tracer, default: Tracer }. You need to make the value meaning of the top-level export = have the Tracer type. You will probably need to make the interface contain only types, so it can merge with a top-level const tracer: Tracer value.
// namespace is type-only (aka uninstantiated)
// made it lowercase since main export is `tracer`
declare namespace tracer {
// export some types only
export interface Foo {}
}
// this type no longer merges, it's uppercase
interface Tracer {
default: Tracer; // "default export" moves to this property
// ...
}
// this value merges with the uninstantiated namespace
declare const tracer: Tracer;
// export value namespace merge
export = tracer;
Thanks for the tip! That suggestion definitely makes sense (i.e. value vs type). I pushed an update and rearranged the diff a bit. It should be easier to read now with the "Hide Whitespace" setting enabled.
After that update, it seems it doesn't understand I want to merge the tracer const with the tracer namespace. I'm getting this error (similar error on the namespace declaration):
It looks like the namespace must still contain a value.
You were right — sneaky one!