uuidjs / uuid

Generate RFC-compliant UUIDs in JavaScript
MIT License
14.62k stars 902 forks source link

[BUG] `Unexpected token 'export'` with Jest (on Cloudflare Workers) #678

Closed AlexErrant closed 2 months ago

AlexErrant commented 1 year ago

Before you begin...

Description of the problem

This issue is very similar to https://github.com/uuidjs/uuid/issues/451 - except I'm on v9 of this library, which is supposed to fix this issue. I'm also on Jest v29.3.1. I'm also also using Cloudflare workers - but I'm not sure how that influences this problem. The moduleNameMapper workaround still works.

Stacktrace:

 C:\Code\miniflare-typescript-esbuild-jest\node_modules\uuid\dist\esm-browser\index.js:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,jest){export { default as v1 } from './v1.js';
                                                                                      ^^^^^^

    SyntaxError: Unexpected token 'export'

      at Runtime.createScriptFromCode (node_modules/jest-runtime/build/index.js:1449:14)
          at async Promise.all (index 0)

Recipe for reproducing

I made three small commits off the official cloudflare+jest example repo here which demonstrates the problem.

Environment

System: OS: Windows 10 10.0.19045 CPU: (12) x64 Intel(R) Core(TM) i7-10850H CPU @ 2.70GHz Memory: 12.58 GB / 63.73 GB Binaries: Node: 18.12.1 - C:\Program Files\nodejs\node.EXE Yarn: 1.22.19 - C:\Program Files (x86)\Yarn\bin\yarn.CMD npm: 8.19.2 - C:\Program Files\nodejs\npm.CMD Browsers: Edge: Spartan (44.19041.1266.0), Chromium (108.0.1462.76) Internet Explorer: 11.0.19041.1566 npmPackages: @cloudflare/workers-types: ^3.11.0 => 3.11.0 @types/jest: 29.2.5 => 29.2.5 @types/uuid: ^9.0.0 => 9.0.0 esbuild: ^0.14.41 => 0.14.41 jest: 29.3.1 => 29.3.1 jest-environment-miniflare: ^2.5.0 => 2.5.0 miniflare: ^2.5.0 => 2.5.0 prettier: ^2.6.2 => 2.6.2 ts-jest: 29.0.4 => 29.0.4 typescript: ^4.7.2 => 4.7.2 uuid: ^9.0.0 => 9.0.0 wrangler: ^2.0.7 => 2.0.7

tomi-bigpi commented 1 year ago

I think this is due to a missing "type": "module" in package.json. However, adding that has other side effects in the repo, so using explicit .cjs and .mjs file extensions should be more robust across all the various use cases. https://nodejs.org/api/packages.html#dual-commonjses-module-packages has some good details. If I can spare the time, I may try to put together a PR with those changes, unless other fixes are already planned?

EDIT: I had a quick look and there are a lot of side effects in the project from adding "type": "module" which quickly spiraled into making sure Common JS files have a .cjs extension etc. which had a lot of downstream implications that I don't have time to sort out (nor have enough context to not break the various consumers out there). We'll locally patch package.json in our project for the time being to be able to move forward.

cdauth commented 1 year ago

The problem seems to be that Jest resolves the main file by using the exports['.'].browser.import property in the package.json of uuid, but that file is interpreted as CommonJS despite containing ESM code. This can be solved by renaming it to .mjs, which I have documented in #692.

As a workaround, I specified this in my Jest config:

   moduleNameMapper: {
        '^uuid$': '<rootDir>/node_modules/uuid/wrapper.mjs'
    }
slhck commented 1 year ago

Hmm, even when doing this, I get:

    Details:

    .../node_modules/uuid/wrapper.mjs:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,jest){import uuid from './dist/index.js';
                                                                                      ^^^^^^

    SyntaxError: Cannot use import statement outside a module

I use:

  "jest": {
    "preset": "ts-jest",
    "setupFiles": [],
    "testEnvironment": "node",
    "testMatch": [
      "**/test/*.test.ts"
    ],
    "moduleNameMapper": {
      "^uuid$": "<rootDir>/node_modules/uuid/wrapper.mjs"
    }
  },
cdauth commented 1 year ago

Hm, it sounds pretty unusual that your setup would specifically pick the MJS file but then interpret it as CJS. But then in your case I guess the require.resolve() workaround or using "^uuid$": "<rootDir>/node_modules/uuid/dist/index.js" would be the right workaround.

Personally I am using Jest in an ESM project, so I am using the ts-jest ESM setup. So I'm using the preset ts-jest/presets/default-esm, have the ts-jest useESM global set to true and run Jest using NODE_OPTIONS=--experimental-vm-modules jest. There I cannot use require.resolve(), and importing CJS files brings lots of trouble.

slhck commented 1 year ago

Ah, maybe it is because this is not an ESM project to begin with. I have another ESM project where I can run tests without issues, but I cannot migrate this project to ESM (yet).

Edit: FWIW, I vendored the dependency, as I only needed parts of the package.

github-actions[bot] commented 1 year ago

Marking as stale due to 90 days with no activity.

AlexErrant commented 1 year ago

Go away stalebot.

zhouyali commented 9 months ago
'^uuid$': '<rootDir>/node_modules/uuid/wrapper.mjs'

I tried this config, and It cause another problem ’RangeError: Maximum call stack size exceeded‘

zhouyali commented 9 months ago

111

stoutatnerdery commented 9 months ago

@zhouyali this worked for me: uuid: require.resolve('uuid'),

bjmc commented 7 months ago

Hello, we're seeing (I believe) this same problem.

I tried to apply the moduleNameMapper workaround described above but then I wind up with a different error:

Must use import to load ES Module: /home/bjmc/Sandbox/ska-oso-odt-ui/node_modules/uuid/wrapper.mjs

FWIW, we are not using/importing uuid directly: one of our dependencies is including it.

Any advice or suggestions would be greatly appreciated! Should we be adopting ts-jest (as mentioned by @cdauth) if we're testing a React/Typescript project?

broofa commented 5 months ago

Any chance #736 helps with this? (asking only because CFWorkers are mentioned there. I don't have time to investigate this in depth at the moment, as I'm focused on rolling a new release for RFC9562)

broofa commented 2 months ago

@AlexErrant et al: I've just published uuid@11 (prerelease) as uuid@beta, which appears to fix this issue.

The caveat here is that I didn't deliberately set out to fix this as part of v11. It's more that v11 was a substantial rewrite that included switching the source to TypeScript, and switching the transpiler from babel to tsc for builds, among other things.

Basically I'm not too surprised this issue has gone away, but nor can I tell you why it went away.

tl;dr: I'm going to close this out because it solve's Alex's issue, but I can't say if it'll solve the issue for the rest of you.

Please do test out the new version and let me know if you see issues.


Verifying issue exists using Alex's example repo:

$ git clone https://github.com/AlexErrant/miniflare-typescript-esbuild-jest.git

$ cd miniflare-typescript-esbuild-jest

$ npm install  # this installs uuid@9

$ npm test
# <snip>
# ....
    /private/tmp/foo/miniflare-typescript-esbuild-jest/node_modules/uuid/dist/esm-browser/index.js:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,jest){export { default as v1 } from './v1.js';

Testing with uuid@10 release, same issue:

$ npm install uuid@10

$ npm test
# <snip... same error as above>

Verifying issue is fixed in v11 (prerelease):

$ npm install uuid@beta

$ npm test
# <snip>
 PASS  test/index.spec.ts
 PASS  test/response.spec.ts
 PASS  test/counter.spec.ts