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

TS1203: Export assignment cannot be used when targeting ECMAScript modules. Consider using 'export default' or another module format instead. #2919

Closed
Maxim-Mazurok opened this issue Mar 18, 2023 · 25 comments

Comments

@Maxim-Mazurok
Copy link
Contributor

Maxim-Mazurok commented Mar 18, 2023

Describe the bug
Getting the following error:

tsc --noEmit        
../bla/node_modules/mathjs/types/index.d.ts:5:1 - error TS1203: Export assignment cannot be used when targeting 
ECMAScript modules. Consider using 'export default' or another module format instead.

5 export = math
  ~~~~~~~~~~~~~


Found 1 error in ../balances/node_modules/mathjs/types/index.d.ts:5

To Reproduce
I have "type": "module" in my package.json and

"target": "ESNext",
"module": "ESNext",
"moduleResolution": "Node16"

in my TSConfig, using TS v5.0.2, it used to work with TS v4.9.5

@Maxim-Mazurok
Copy link
Contributor Author

Not entirely sure why, but using "moduleResolution": "bundler" instead of Node16 resolved the issue

@josdejong
Copy link
Owner

Thanks for bringing this up. So in index.d.ts I see:

declare const math: math.MathJsStatic
export as namespace math
export = math

For reference: https://stackoverflow.com/questions/48708410/typescript-error-an-export-assignment-cannot-be-used-in-a-module-with-other-exp

Anyone able to see if we can/should improve on this?

@Maxim-Mazurok
Copy link
Contributor Author

Maxim-Mazurok commented Mar 25, 2023

No worries :)

Not sure, I've tried the approach from the answer:

import { Decimal } from 'decimal.js'

type NoLiteralType<T> = T extends number
  ? number
  : T extends string
  ? string
  : T extends boolean
  ? boolean
  : T

declare module "math" {
  namespace math {
    //...
  }
  export = math;
}

And it resulted in the same error as in OP: ../../../../node_modules/mathjs/types/index.d.ts(6424,3): error TS1203: Export assignment cannot be used when targeting ECMAScript modules. Consider using 'export default' or another module format instead.

@Maxim-Mazurok
Copy link
Contributor Author

Also npm packages like csv-parser and moment seem to be using the same syntax as mathjs but no errors when trying to use and compile with old config... Maybe because they're both exporting a function instead of an object, not sure...

@josdejong
Copy link
Owner

Thanks for giving it a try Maxim. From what I understand from it, we should get rid of the export = math since that results in a CommonJS module, and instead use export default math to get a modern ES module. I briefly tried that but get a lot of errors then when running the tests of the TypeScript definitions.

@Maxim-Mazurok
Copy link
Contributor Author

Hey @josdejong
I did get errors you mentioned and came up with two potential solutions:

  1. Fix for #2919 option 1 #2931
  2. Fix for #2919 option 2 #2932

Option 1 potentially breaks integrations and changes public API. Option 2 is safer, but ideally requires automation. I couldn't find a way to replicate previous behavior, my guess is that such broad ambiguous exports are not in ESM philosophy so likely aren't supported. I just ran a few regexes to extract exports, two of them trigger eslint errors, and there's also import() method that can't be exported at all as far as I can tell.

@Maxim-Mazurok
Copy link
Contributor Author

This also might help: https://arethetypeswrong.github.io/?p=mathjs%4011.8.0

@josdejong
Copy link
Owner

Hmm. So both potential solutions are not ideal yet. Thanks for working them out!

Do you have any understanding on why export default math would not work?

Would it work to remove the namespace and only export the interface MathJsStatic? (and maybe rename it to MathJs)

@Maxim-Mazurok
Copy link
Contributor Author

I'm sorry, I've completed the transition to CJS from ESM because NX doesn't have proper support for ESM yet, so this issue is no longer relevant to me, and a good solution isn't apparent to me, so I hope someone else can pick it up from here, unsubscribing for now, cheers!

@josdejong
Copy link
Owner

Good to hear you have a solution for your case. Thanks for your work on solving this!

I'll close your two PR's for the time being until someone can pick this up.

@nalply
Copy link

nalply commented Jun 26, 2023

I will try to pick this up. Let's hope that I find a solution.

@josdejong
Copy link
Owner

Thanks for your offer @nalply ! Maxim did explore a couple of approaches already, that may serve as a good start.

@nalply
Copy link

nalply commented Jun 26, 2023

Option 2 compiles fine for me, however the generated file in lib/esm/type/complex/Complex.js has SyntaxError: ambiguous indirect export: default, because of this line:

https://github.com/josdejong/mathjs/blob/develop/src/type/complex/Complex.js#L1

I will need to clone mathjs and tinker a bit.

Please tell me if I am barking up the wrong tree.

@nalply
Copy link

nalply commented Jun 26, 2023

Option 2 seems to work for me, but I had to solve the problems with complex.js and fraction.js first to continue. However my use case is different again: I want to program in TypeScript for the browser without bundling. I have some free time to develop an RPN calculator., it's a hobby. Because I hate bundling, I am stubborn in not allowing bundling during development.

I am using only a small part of mathjs, so I won't give a recommendation to use option 2.

And the problem with "ambiguous indirect export": it is because Firefox expected an ES module and the Node trick to import CommonJS into ES module does not work in the browser. I cloned fraction.js and made it an ES module by making the IIFE return the class and havingexport default <IIFE>.

I know that this change is a breaking one and won't suggest it. But for my RPN calculator this is a quick, nice change.

EDIT: Update what I did

@josdejong
Copy link
Owner

I want to program in TypeScript for the browser without bundling.

🤔 hm are you trying to load non-bundled code in the browser? I'm not sure but I guess there are two different issues in play then: one is the export = math issue. And the other is that some of the dependencies are not ES modules, making it currently impossible to load the non-bundled code in a browser, see #1841, #1928. In this issue (#2919) we try to tackle the export = math issue

@nalply
Copy link

nalply commented Jun 28, 2023

[...] I guess there are two different issues in play then: one is the export = math issue. And the other is that some of the dependencies are not ES modules [...]

Yeah!

I forked complex.js and fraction.js yesterday and all is well, development has been ensuing since then rather smoothly. I am using https://modern-web.dev/docs/dev-server/overview/ which compiles TypeScript on the fly, and I hacked a small plugin to do some more on-the-fly processing and this makes me happy. YOLO.

@josdejong
Copy link
Owner

😂 👍

@imccausl
Copy link

As a temporary workaround until this issue is fixed, one way we were able to solve it was by doing this. We're suppressing the typescript error, but the types will still show up correctly.

// @ts-expect-error suppress typescript errors until issue is resolved https://github.com/josdejong/mathjs/issues/2919
const mathjs: typeof import('mathjs') = require('mathjs')

@hfhchan-plb
Copy link

hfhchan-plb commented Aug 26, 2023

This is partly because TypeScript expects you to ship two different .d.ts files, one for cjs, and one for esm.

See: https://www.typescriptlang.org/docs/handbook/esm-node.html#packagejson-exports-imports-and-self-referencing

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. Every declaration file is interpreted either as a CommonJS module or as an ES module, based on its file extension and the "type" field of the package.json, and this detected module kind must match the module kind that Node will detect for the corresponding JavaScript file for type checking to be correct. Attempting to use a single .d.ts file to type both an ES module entrypoint and a CommonJS entrypoint will cause TypeScript to think only one of those entrypoints exists, causing compiler errors for users of the package.

As of now, the package.json looks like this:

    ".": {
      "types": "./types/index.d.ts",
      "import": "./lib/esm/index.js",
      "require": "./lib/cjs/index.js"
    },

It should look like this:

    ".": {
      "import": "./lib/esm/index.js",
      "require": "./lib/cjs/index.js"
    },

with the index.d.ts copied into ./lib/esm/ and ./lib/cjs, or:

    ".": {
      "import": {
        "types": "./types/index.d.ts",
        "default": "./lib/esm/index.js"
      }
      "require": {
        "types": "./types/index.d.cts",
        "default": "./lib/cjs/index.js"
      }
    },

with index.d.ts duplicated to index.d.cts.

No other changes are necessary.

josdejong added a commit that referenced this issue Aug 30, 2023
@josdejong
Copy link
Owner

O wow, that easy? That would be great!

@hfhchan-plb so can the index.d.cts contain import { Decimal } from 'decimal.js' and export ... then? I.e. shouldn't it be refactored to actual CJS use require and module.exports?

I've prepared a PR in #3021, anyone able to have a look or even better: verify whether this PR solves the issue?

@hfhchan-plb
Copy link

hfhchan-plb commented Sep 1, 2023

so can the index.d.cts contain import { Decimal } from 'decimal.js' and export ... then?

Yes. The import / export syntax in .d.cts files is the TypeScript import syntax, not the ESM one.

I.e. shouldn't it be refactored to actual CJS use require and module.exports?

No. If you do that, then TS will start throwing errors. 🤷

@hfhchan-plb
Copy link

I've prepared a PR in #3021, anyone able to have a look or even better: verify whether this PR solves the issue?

LGTM

@josdejong
Copy link
Owner

Thanks for checking it out! Will merge it soon.

@josdejong
Copy link
Owner

josdejong commented Oct 12, 2023

I've done some testing with #3021, having alternative entry points in the package.json file. This does not solve the issue unfortunately.

So I think we should resort to one of the options worked out by Maxim, see #2919 (comment)). I think the second option is best since it does not need any special tricks: it is just the dumb way to define that function create returns a mathjs instance, and that the library exposes a static instance too. We have to define it twice. That is unfortunate and I agree with Maxim that we should automate that. In the long term I want to autogenerate all type definitions, but for the time being we will have to maintain it manually.

I've now created an new PR #3079 using the approach of option 2 of Maxim, and added documentation. Still need to do testing and some last thingies. Feedback would be welcome.

@josdejong
Copy link
Owner

Fixed now in v12.0.0 via #3079

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

Successfully merging a pull request may close this issue.

5 participants