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

Upgrade prettier as a dev dependency breaks non ESM react projects #15098

Closed
dominikzogg opened this issue Jul 14, 2023 · 7 comments
Closed

Upgrade prettier as a dev dependency breaks non ESM react projects #15098

dominikzogg opened this issue Jul 14, 2023 · 7 comments
Labels
locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. scope:external This is not an issue with Prettier, it’s an issue with external software, like an editor integration

Comments

@dominikzogg
Copy link

dominikzogg commented Jul 14, 2023

Environments:

  • Prettier Version: 3.0.0
  • Running Prettier via: npm run cs (prettier --check bin ..folders *.js *.ts)
  • Runtime: v18.16.1
  • Operating System: Rocky Linux 9.2 (Blue Onyx) x86_64
  • Prettier plugins (if any): none

Steps to reproduce:

Create commonjs variant of a react application using prettier 2.x as a dev dependency => works.
Then upgrade to version 3.0 and then receive the following errors for example running tests:

/app/node_modules/jest-runtime/build/index.js:2297
    throw new Error(message);
          ^

Error: You need to run with a version of node that supports ES Modules in the VM API. See https://jestjs.io/docs/ecmascript-modules
    at invariant (/app/node_modules/jest-runtime/build/index.js:2297:11)
    at importModuleDynamically (/app/node_modules/jest-runtime/build/index.js:1502:11)
    at importModuleDynamicallyWrapper (node:internal/vm/module:429:21)
    at importModuleDynamicallyCallback (node:internal/process/esm_loader:35:14)
    at Object.<anonymous> (/app/node_modules/prettier/index.cjs:600:23)
    at Runtime._execModule (/app/node_modules/jest-runtime/build/index.js:1430:24)
    at Runtime._loadModule (/app/node_modules/jest-runtime/build/index.js:1013:12)
    at Runtime.requireModule (/app/node_modules/jest-runtime/build/index.js:873:12)
    at Runtime.requireModuleOrMock (/app/node_modules/jest-runtime/build/index.js:1039:21)
    at Object.<anonymous> (/app/tests/utils/formatter.ts:11:19)
    at Runtime._execModule (/app/node_modules/jest-runtime/build/index.js:1430:24)
    at Runtime._loadModule (/app/node_modules/jest-runtime/build/index.js:1013:12)
    at Runtime.requireModule (/app/node_modules/jest-runtime/build/index.js:873:12)
    at Runtime.requireModuleOrMock (/app/node_modules/jest-runtime/build/index.js:1039:21)
    at Object.<anonymous> (/app/tests/unit/user/template.test.tsx:8:20)
    at Runtime._execModule (/app/node_modules/jest-runtime/build/index.js:1430:24)
    at Runtime._loadModule (/app/node_modules/jest-runtime/build/index.js:1013:12)
    at Runtime.requireModule (/app/node_modules/jest-runtime/build/index.js:873:12)
    at jestAdapter (/app/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapter.js:77:13)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at runTestInternal (/app/node_modules/jest-runner/build/runTest.js:367:16)
    at runTest (/app/node_modules/jest-runner/build/runTest.js:444:34)
    at Object.worker (/app/node_modules/jest-runner/build/testWorker.js:106:12)

Expected behavior:

I would expect it would not break other code, when upgrading from 2.x to 3.0.0.
As you can see this in the only change after i ran

npm i -D prettier@^3.0.0
-      "version": "2.8.8",
-      "resolved": "https://registry.npmjs.org/prettier/-/prettier-2.8.8.tgz",
-      "integrity": "sha512-tdN8qQGvNjw4CHbY+XXk0JgCXn9QiF21a55rBe5LJAU+kDyC4WQn4+awm2Xfk2lQMk5fKup9XgzTZtGkjBdP9Q==",
+      "version": "3.0.0",
+      "resolved": "https://registry.npmjs.org/prettier/-/prettier-3.0.0.tgz",
+      "integrity": "sha512-zBf5eHpwHOGPC47h0zrPyNn+eAEIdEzfywMoYn2XPi0P44Zp0tSq64rq0xAREh4auw2cJZHo9QUob+NqCQky4g==",
       "dev": true,
       "bin": {
-        "prettier": "bin-prettier.js"
+        "prettier": "bin/prettier.cjs"
       },
       "engines": {
-        "node": ">=10.13.0"
+        "node": ">=14"
       }

Actual behavior:

I breaks for example jest tests.

@kachkaev
Copy link
Member

👋 @dominikzogg! I think that this is an issue with Jest which is being tracked at jestjs/jest#13495. Please correct me if I am wrong and we will re-open this issue.

@kachkaev kachkaev closed this as not planned Won't fix, can't repro, duplicate, stale Jul 14, 2023
@kachkaev kachkaev added the scope:external This is not an issue with Prettier, it’s an issue with external software, like an editor integration label Jul 14, 2023
@dominikzogg
Copy link
Author

@kachkaev thanks for the fast response. I can't tell jest and prettier 3 in a pure backend project works. I probably have to look more deeply into esm, even when i dislike what i know of (no dynamic import, which i need in some projects), to understand the real issue.

@auvred
Copy link
Contributor

auvred commented Jul 15, 2023

Hi @dominikzogg! I recently ran into a similar issue, the another way to solve this is to use jest-light-runner. It doesn't require experimental vm api

Prettier also uses it:

https://github.com/prettier/prettier/blob/79d8a3983842d1d8a077f177ed8b983c1c596088/jest.config.js#L57C12-L57C29

@nicolaschambrier
Copy link

Hi 👋 I believe the issue here and Jest's difficulties to upgrade to Prettier v3 are not related, and it would be worth re-opening this issue @kachkaev

The issue here is that the generated prettier/index.cjs at line 600 includes this line:

var prettierPromise = import("./index.mjs");

which comes from there: https://github.com/prettier/prettier/blob/main/src/index.cjs#L3

If you export a .cjs file with calls to import() it will not work well 😅 unless we enable --experimental-vm-modules.

We need to have an answer on our side so we know if we need to enable this flag or if we can wait for a fix or your exported build?

@kachkaev
Copy link
Member

AFAIR, dynamic import() statement work in Node.js without any flags, but I might be wrong. The line you are referring to comes from #13266 by @fisker. Maybe he can clarify the details but I still believe that the problem is external to Prettier.

@nicolaschambrier
Copy link

🤔 oh you're totally right, just tested this with no flag and it totally works. Gotta learn something every day 😅
So it's most likely a mischeck in Jest, yes. We also have build issues but they might be related to something else and will open another issue if relevant.

Thanks for your quick answer!

@fisker
Copy link
Member

fisker commented Jul 25, 2023

It's broken since jest runs in vm, and it requires --experimental-vm-modules to use import(). https://jestjs.io/docs/ecmascript-modules

@github-actions github-actions bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Oct 24, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. scope:external This is not an issue with Prettier, it’s an issue with external software, like an editor integration
Projects
None yet
Development

No branches or pull requests

5 participants