Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using 'seperate' declaration with CommonJS creates invalid types #327

Open
jonkoops opened this issue Aug 1, 2023 · 6 comments
Open

Using 'seperate' declaration with CommonJS creates invalid types #327

jonkoops opened this issue Aug 1, 2023 · 6 comments

Comments

@jonkoops
Copy link

jonkoops commented Aug 1, 2023

Description

When building an NPM library that supports both ESM and CommonJS, and has the types separated, an invalid exports is generated:

"exports": {
  ".": {
    "import": {
      "types": "./types/test.d.ts",
      "default": "./esm/test.js"
    },
    "require": {
      "types": "./types/test.d.ts",
      "default": "./script/test.js"
    }
  }
},

Here, both of the types fields refer to the same file, but according to the TypeScript Handbook this is not allowed:

It’s important to note that the CommonJS entrypoint and the ES module entrypoint each needs its own declaration file, even if the contents are the same between them.

Steps to reproduce

Create a file called test.ts and add some dummy content with an export:

export const message = "Hello World!";

Create a build script under scripts/build_npm.ts with the following contents:

import { build, emptyDir } from "https://deno.land/x/dnt/mod.ts";

await emptyDir("./npm");

await build({
  entryPoints: ["./test.ts"],
  outDir: "./npm",
  declaration: "separate",
  scriptModule: "cjs",
  shims: {
    deno: true,
  },
  package: {
    name: "your-package",
    version: Deno.args[0],
  },
});

Then, build the NPM package with the following command:

deno run -A scripts/build_npm.ts 0.1.0

Once the package has been built, it can be inspected using the Are the types wrong? CLI:

attw --pack ./npm

Which should show the following error:

A screenshot showing the checks for the "Are the types wrong? CLI" failing.

Solution

As per the suggestion in the TypeScript Handbook, a second file should be added specifically for the CommonJS version and linked from the types field.

@dsherret
Copy link
Member

dsherret commented Aug 1, 2023

Yeah, don't use "separate". Instead use "inline" which is the default.

declaration: "separate", was kept because it used to be how dnt emitted the types, but it was switched away from for this reason. It's now mostly undocumented other than existing in the type declarations. What I will do is add a warning when this is used for the time being.

@jonkoops
Copy link
Author

jonkoops commented Aug 1, 2023

Another issue somewhat related to this, is that the type definitions that are emitted for CommonJS are not really correct. For example, given the aforementioned input the following type is generated:

export declare const message = "Hello World!";

This is of course correct for the ESM bundle, but for CommonJS I would expect something like the following:

declare const message = "Hello World!";

export = { message };

@dsherret
Copy link
Member

dsherret commented Aug 1, 2023

@jonkoops that's the typescript compiler giving that output. I don't believe there's a way to get it to do that, but let me know if you know a way. I believe that output should work fine though.

@jonkoops
Copy link
Author

jonkoops commented Aug 1, 2023

Yeah, I think the only way to do so would be to convert the source code to CommonJS as well. There really is no way for the TypeScript compiler to emit valid CommonJS types from an ESM source, as far as I know.

Perhaps the best solution here is to simply not link any types to the CommonJS input, which I believe is the default behavior with "inline" anyways.

@phaux
Copy link

phaux commented Oct 3, 2023

Btw. esm.sh doesn't seem to detect types when emitted with "inline", but does with "separate".

(yes, I'm publishing a deno module to npm and load it back into deno via esm.sh)

@dsherret
Copy link
Member

dsherret commented Oct 3, 2023

If the package works with vanilla tsc then I'd recommend opening an issue on esm.sh's repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants