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

SyntaxError: Cannot use import statement outside a module, with TypeScript and ES Modules #9860

Closed
dandv opened this issue Apr 22, 2020 · 45 comments · Fixed by #10823
Closed

Comments

@dandv
Copy link
Contributor

dandv commented Apr 22, 2020

🐛 Bug Report

Filing a separate issue at @SimenB's suggestion.

After following the steps to get native ESM support, I'm running into the following error in a project that transpiles TypeScript files using @babel/preset-typescript:

/home/dandv/jets/lib.test.ts:1
import { foo } from './lib';
^^^^^^

SyntaxError: Cannot use import statement outside a module

  at Runtime._execModule (node_modules/jest-runtime/build/index.js:1074:58)

To Reproduce

  1. git clone https://github.com/dandv/jest-typescript-es-modules.git
  2. cd jest-typescript-es-modules
  3. npm install
  4. npm test

Expected behavior

Test passes.

Link to repl or repo (highly encouraged)

https://github.com/dandv/jest-typescript-es-modules

envinfo

System:
    OS: Linux 5.3 Ubuntu 18.04.4 LTS (Bionic Beaver)
    CPU: (8) x64 Intel(R) Core(TM) i7-8665U CPU @ 1.90GHz
  Binaries:
    Node: 13.13.0 - /usr/bin/node
    npm: 6.14.4 - ~/.local/bin/npm
  npmPackages:
    jest: ^25.4.0 => 25.4.0
@SimenB
Copy link
Member

SimenB commented Apr 22, 2020

This is due to this check: https://github.com/facebook/jest/blob/6a0f070e0210eb11408a7c8ebb003ff73d62e420/packages/jest-resolve/src/shouldLoadAsEsm.ts#L51-L55

Not sure how to detect if it's an actual module or should use synthetic modules (like JSON). All current code only expects JS (we feed it to vm.Script), so might make sense to do the same here. If not, we'll need an option so the user can tell us.

@thymikee @jeysal thoughts?

@askirmas
Copy link

Take my transpile config
https://github.com/askirmas/askirmas.github.io/blob/8d1a0f2601c2683597edaa473f3b9ca9068abd72/jest.schema.json#L756-L773

@dandv
Copy link
Contributor Author

dandv commented Apr 25, 2020

@askirmas: doesn't that config transform imports, hence making moot the point of Jest natively supporting ES Modules?

@SimenB
Copy link
Member

SimenB commented Apr 25, 2020

Yup. It's quite simple to fix, but I don't think we want to special case .ts, .tsx or .jsx for that matter. What about people using .coffee, .vue etc.?

Right now the logic is

  • .mjs is always ESM
  • .cjs is always CJS
  • .js is ESM if closest package.json has a type: 'module' field, otherwise CJS
  • all other extensions are CJS

What I'm thinking makes sense is to make that last one behave like js, i.e. infer from the type field. But it might be better to add an option that overrides this check and says "treat ts and tsx as ESM"?

@dandv
Copy link
Contributor Author

dandv commented Apr 25, 2020 via email

@SimenB
Copy link
Member

SimenB commented Apr 25, 2020

Sure, but after transplantation it has to still use import, or require etc is undefined. Might need an option. esmExtensions?

@jeysal
Copy link
Contributor

jeysal commented Apr 25, 2020

'transplantation', nice autocorrect 😅 but with Babel, TS is a completely different thing than CJS, if you use plugin-typescript but not plugin-transform-modules-commonjs, types get stripped but it remains ESM

@SimenB SimenB closed this as completed Apr 25, 2020
@SimenB SimenB reopened this Apr 25, 2020
@SimenB
Copy link
Member

SimenB commented Apr 25, 2020

Hah, yeah. Which is why I think we might need an option instead of forcing or otherwise trying to infer it

@GeoffreyBooth
Copy link

What I'm thinking makes sense is to make that last one behave like js, i.e. infer from the type field.

Hi, I stumbled across this issue and thought I'd say hi. I'm the maintainer of CoffeeScript and I've been dealing with this same issue. In my case, there are plenty of legacy .coffee files out there that use require/CommonJS, including the CoffeeScript codebase itself. My plan is to treat .coffee as equivalent to .js, as in, “behave however a .js file would at this path”—so if it’s in a "type": "module" package scope, it's ESM. I think that's the safest approach; even if you think all TypeScript you'll ever encounter uses import/export, I wouldn't be surprised if there are some folks out there who have been mixing import and require statements occasionally, since presumably they currently work.

@SimenB
Copy link
Member

SimenB commented Apr 27, 2020

Thanks for chiming in! I think that makes sense for us as well - it's what we already do for CJS - we require you to transform whatever you import to CJS before Jest will load it. And if you've opted into ESM for .js files I think it's a pretty safe assumption that you want the same behavior for your ts (or .coffee, .vue) files. Happy to hear people in the modules WG are thinking along the same lines 👍

@ahnpnl
Copy link
Contributor

ahnpnl commented May 9, 2020

What I'm thinking makes sense is to make that last one behave like js, i.e. infer from the type field.

Hi, I stumbled across this issue and thought I'd say hi. I'm the maintainer of CoffeeScript and I've been dealing with this same issue. In my case, there are plenty of legacy .coffee files out there that use require/CommonJS, including the CoffeeScript codebase itself. My plan is to treat .coffee as equivalent to .js, as in, “behave however a .js file would at this path”—so if it’s in a "type": "module" package scope, it's ESM. I think that's the safest approach; even if you think all TypeScript you'll ever encounter uses import/export, I wouldn't be surprised if there are some folks out there who have been mixing import and require statements occasionally, since presumably they currently work.

cc @lmiller1990

Yup. It's quite simple to fix, but I don't think we want to special case .ts, .tsx or .jsx for that matter. What about people using .coffee, .vue etc.?

Right now the logic is

  • .mjs is always ESM
  • .cjs is always CJS
  • .js is ESM if closest package.json has a type: 'module' field, otherwise CJS
  • all other extensions are CJS

What I'm thinking makes sense is to make that last one behave like js, i.e. infer from the type field. But it might be better to add an option that overrides this check and says "treat ts and tsx as ESM"?

@SimenB Is it safe to use this approach for ts-jest as ts-jest only needs to transform ts/tsx to mjs/cjs depending on tsconfig target ?

Maybe only the case when transforming files from node_modules, it is necessary to check type: module in package.json

@ghasemikasra39
Copy link

Getting the same issue

@mnajjarian
Copy link

I had the same problem. I've fixed that by including js files in ts-node:

"transform": { "^.+\\.(ts|tsx|js|jsx)?$": "ts-jest" }

@ghasemikasra39
Copy link

I had the same problem. I've fixed that by including js files in ts-node:

"transform": { "^.+\\.(ts|tsx|js|jsx)?$": "ts-jest" }

Thanks Mahdi jan. Worked for me as well.

@SimenB
Copy link
Member

SimenB commented Oct 23, 2020

My current thinking is that a transformer should return the format of the code it has transpiled.

Right now a transformer returns: https://github.com/facebook/jest/blob/1535af7659e0392b3f7c6124fa58d230907ee38d/packages/jest-types/src/Transform.ts#L9-L14

I'm thinking in addition it can return a moduleFormat?: 'ESM' | 'CJS' which we default to CJS. Thoughts?

A problem with this approach is that it might not be possible for a transformer to know - e.g. with the babel-jest transform shipped by default we can specify to Babel that we support ESM, but the user's Babel config can still transpile to CJS code. So I'm back to perhaps just a top-level configuration option allowing the user to say "all the files should be interpreted as ESM" (via some glob).

(https://en.wikipedia.org/wiki/Rubber_duck_debugging 😅)

@SimenB
Copy link
Member

SimenB commented Oct 25, 2020

No, we won't be passing the result of one transformer into another. What would happen is that a user specifies "moduleFormat": "ESM" and somehow configures vue-jest to return ESM instead of CJS. Jest would then execute the ESM - no CJS transpilation at all

@lmiller1990
Copy link

lmiller1990 commented Oct 25, 2020

I see - I did not realize Jest could execute ESM (I have always been compiling to cjs). Is this via babel-jest?

This would be great, then we can target ESM and not bother with the cjs compilation step (vue-jest already supports both ESM and CJS).

@SimenB
Copy link
Member

SimenB commented Oct 26, 2020

Yeah, I'm currently working (on and off) on adding native ESM support, see #9430. This issue is a small, but important part of that work 🙂

@RowinVanAmsterdam
Copy link

I like that suggestion! We already support passing config to transformers, like so

{
  "transform": {
    "\\.[jt]sx?$": ["babel-jest", { "rootMode": "upward" }]
  }
}

I guess we could have a third argument which is "config for jest"? Then stick moduleFormat in there.

{
  "transform": {
    "\\.[jt]sx?$": [
      "babel-jest",
      { "rootMode": "upward" },
      { "moduleFormat": "ESM" }
    ]
  }
}

We could also go for your suggestion of accepting an object - simple enough to normalize the user config into that as well.

{
  "transform": {
    "\\.[jt]sx?$": {
      "transformer": "babel-jest",
      "transformerOptions": { "rootMode": "upward" },
      "moduleFormat": "ESM"
    }
  }
}

I tried your suggestion, but unfortunately no luck for me yet. Project consists of .mjs, and commonJS. I added the transform in the package.json file and I receive the error on this line of code:
console.log(In ${window.location.href} starting script: ${import.meta.url})

I have to remove above line and add this part of code in the jest.config.file before my tests finally pass:

transform: {
    "^.+\\.jsx?$": "babel-jest",
    "^.+\\.mjs$": "babel-jest",
  }

As soon as I remove above transform and use your suggestion, it will throw me the same syntax error.

@SimenB
Copy link
Member

SimenB commented Oct 26, 2020

Yeah, that's suggested syntax, it's not implemented yet. Will be included in Jest 27 which will come sometime before Christmas (hopefully earlier, but I don't wanna make any promises I'm not able to keep)

@SimenB
Copy link
Member

SimenB commented Nov 14, 2020

OK, I've changed my mind (again) - I'm introducing a new top-level option called extensionsToTreatAsEsm - there you can pass e.g. .ts to have all TS files interpreted as ESM. Transformers will be called with supportsStaticESM: true which it can use to toggle import/export on and off.

Other behavior will remain as is - mjs is always ESM, cjs is always CJS and js will be interpreted based on type field in its closest package.json. This new option will only affect other extensions

@ahnpnl
Copy link
Contributor

ahnpnl commented Nov 14, 2020

What is the advantage over the previous approach ? Is this top level option a part of global jest config or is it a part of transform config ?

@SimenB
Copy link
Member

SimenB commented Nov 14, 2020

part of the Jest project config. This is passed to transformers, so you can use it if you want. I'd recommend relying on the passed supportsStaticESM tho as we might come via a require call

@SimenB
Copy link
Member

SimenB commented Nov 14, 2020

@ahnpnl PR: #10823.

@SimenB
Copy link
Member

SimenB commented Dec 5, 2020

I've published jest@next now, please give it a whirl! 🙂

@ahnpnl

This comment has been minimized.

@SimenB

This comment has been minimized.

@SimenB

This comment has been minimized.

@ahnpnl

This comment has been minimized.

@SimenB

This comment has been minimized.

@SimenB

This comment has been minimized.

@ahnpnl

This comment has been minimized.

@ahnpnl

This comment has been minimized.

@SimenB

This comment has been minimized.

@ahnpnl

This comment has been minimized.

@oarthursilva
Copy link

solved by adding "module": "commonjs" in tsconfig.json.

Basically, import modules are available on commonjs

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.