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

[Feature] Add a way to handle CJS/ESM module hell #23662

Open
segevfiner opened this issue Jun 12, 2023 · 34 comments
Open

[Feature] Add a way to handle CJS/ESM module hell #23662

segevfiner opened this issue Jun 12, 2023 · 34 comments

Comments

@segevfiner
Copy link

With the current sorry state of the JavaScript ecosystem, an ongoing struggle is the CJS/ESM modules hell. Whereby, importing CJS from ESM, or ESM from CJS. is troublesome in plain Node, and only really works well in bundlers. Sadly, many things will force you to use CJS (Due to not supporting ESM), and many many modules are ESM only, or otherwise CJS with no compatibility for Node's ESM named import rules, or file extension/no directory imports requirements.

An example is this https://github.com/segevfiner/playwright-ct-vue-on-issue/blob/main/src/components/__tests__/esm-hell.pw.ts

This test imports a TypeScript file that imports lodash-es which is ESM only. This fails in Playwright without "type": "module" set as you can't require an ESM module. With "type": "module", you then hit a truck load of other issues with the poor state of Node's native ESM mode. All imports in the entire code base, including in dependencies, must be adjusted to include file extensions, which TypeScript doesn't seem to check for, except with "module/moduleResolution": "node16" but that mode is troublesome with bundlers. Further more, many CJS modules are not compatible with named imports from ESM as they don't define their exports in a statically analyzable way, this again results in aggravating failures when trying to import them in Node's ESM mode.

To handle this hellish nightmare, I found myself having to manually configure test runners to transform modules that have trouble importing in either CJS or ESM mode, depending on what the test runner uses, and to use tsx to run scripts as it transforms the modules using esbuild to the proper format before Node.

It would be very helpful if @playwright/test like Jest and Vitest, had a way to transform such problems modules too. When running in CJS mode, transform ESM modules to CJS, and vice versa when running in ESM mode, so that we are not limited by the annoying limitations of Node, which bundlers are not limited by, and can focus on actually writing code rather than fighting in the unholy CJS vs ESM purity war.

In Jest this is done by configuring a negative lookahead regexp in transformIgnorePatterns, and configuring your transform to perform the needed conversion, e.g. Babel's ESM to CJS.

In Vitest you sometimes need to configure deps.inline, including all modules that lead to such a problem module, as Vitest doesn't transform after an inner import/require of a non-transformed module. (It's not a require/loader hook like Jest ATM).

@dgozman
Copy link
Contributor

dgozman commented Jun 13, 2023

@segevfiner Thank you for the issue! The situation with CJS/ESM is not ideal indeed. We have recently made some changes to make things easier, but there is definitely room for improvement.

  • Playwright aligned with --moduleResolution=bundler, so you can now omit file extensions in esm mode, see [Feature] Support TypeScript's --moduleResolution bundler #22169. Perhaps this could make it easier for you to switch to ESM completely?
  • You can opt-out from processing certain files with TestConfig.build.external option.
  • Playwright does not transpile anything in the node_modules today. I guess this is the reason lodash-es does not work for you. We can think about lifting this restriction though.
  • Since we do not pre-compile beforehand, but instead transpile in runtime, we are limited by Node.js decisions to load a file as ESM vs CJS depending on the type property in package.json and/or file extension. I am not sure whether tsx works around this, but I guess we can take a look.

Overall, Playwright mostly assumes it executes the Node.js program, not the frontend code that is only written to be consumed by carefully configured bundlers. However, we are open to accommodate some setups. If you have some specific asks, let us know and we'll take a look. Otherwise, the landscape is very diverse, and we won't be able to universally handle it in a single way.

@brady-man-847
Copy link

So there is no way to resolve lodash-es esm support ?! ㅠ

@Smrtnyk
Copy link

Smrtnyk commented Jun 15, 2023

So there is no way to resolve lodash-es esm support ?! ㅠ

quick google says
to import ESM into CommonJS you'll use the asynchronous import() function. The returned promise resolves to an object with a default field (which points to the default exported value), plus a field per any named export

What we did in our repo is that we created a pnpm monorepo, and have separate package just for playwright tests that we made as a module, so we can benefit from all of the esm features

@segevfiner
Copy link
Author

It's actually vanilla lodash that fails for me, as it is not compatible with named exports when imported from Node ESM. (The static analysis part of CJS modules).

If we had a flag like in vitest to force Playwright to transpile it instead of importing it directly

@segevfiner
Copy link
Author

So to summarize:

  • A CJS package that doesn't support named exports in ESM, will fail in ESM mode, e.g. lodash.
  • An ESM only package will fail in CJS mode, e.g. lodash-es

And that's why we can't have nice things...

@dgozman dgozman self-assigned this Jun 16, 2023
@dgozman dgozman added v1.36 and removed triaging labels Jun 16, 2023
@dgozman
Copy link
Contributor

dgozman commented Jun 16, 2023

@segevfiner I've tried your sample repository, and changing lodash-es to lodash makes it work for me. I guess your actual issue has more to it, because in the small repository nothing forces ESM, so using CJS everywhere works fine.

A CJS package that doesn't support named exports in ESM, will fail in ESM mode, e.g. lodash.

What forces the ESM mode for you? I don't see type: module nor any .mjs/.mts files.

@segevfiner
Copy link
Author

Not in this repo. But I tried to enable ESM mode for some other project because some other package than lodash-es is ESM only in that project.

Basically when I have one package that has problem with named exports in ESM mode, and another one that is ESM only, you have an issue with getting either mode working properly.

@segevfiner
Copy link
Author

I have hit another issue where Playwright fails to import lowlight/lib/all, seems like the no extensions transform is failing for that module.

Error: Cannot find module '/Users/segevfiner/junk/playwright-ct-vue-on-issue/node_modules/lowlight/lib/all' imported from /Users/segevfiner/junk/playwright-ct-vue-on-issue/src/components/__tests__/CodeBlock.pw.ts

See: https://github.com/segevfiner/playwright-ct-vue-on-issue/blob/lowlight-issue/src/components/__tests__/CodeBlock.pw.ts

@dgozman

@Pauan
Copy link

Pauan commented Jul 31, 2023

@dgozman Overall, Playwright mostly assumes it executes the Node.js program, not the frontend code that is only written to be consumed by carefully configured bundlers.

That makes no sense. The entire point of Playwright is to write unit tests for browser code, that's why Playwright runs the tests in a headless browser. So it should not be assuming that it is executing Node.js code!

Playwright should be following bundler conventions, not Node.js conventions, because it is running browser code, not Node.js code. And browser code uses bundlers.

If somebody wants to unit test Node.js code, they shouldn't be using Playwright in the first place, because Playwright runs the test code in the browser, not Node.js. So it makes no sense for Playwright to be catering to the Node.js behavior.

@MindaugasMateika
Copy link
Contributor

because Playwright runs the test code in the browser, not Node.js

that's not exactly true

@Pauan
Copy link

Pauan commented Aug 1, 2023

@MindaugasMateika Could you please explain more? Under what circumstances does Playwright run the user's unit test code in Node.js?

@MindaugasMateika
Copy link
Contributor

@MindaugasMateika Could you please explain more? Under what circumstances does Playwright run the user's unit test code in Node.js?

I think pretty much whole code runs on Node.js process and commands are sent to browsers over web-sockets to drive the automation on browser side. So the tests code is more like Node.js program than something else.

@rggammon
Copy link
Member

rggammon commented Aug 8, 2023

If you have some specific asks, let us know and we'll take a look. Otherwise, the landscape is very diverse, and we won't be able to universally handle it in a single way.

I have a playwright + lighthouse 10 + custom Audit setup. Lighthouse takes the module name for the module that contains the CustomAudit class & imports it.

My "CustomAudit" class extends Audit defined by lighthouse (which is an esm module).

I can get this to work if I define CustomAudit in a .mjs file, where I can statically import { Audit } from 'lighthouse'; and derive from it, eg: export default class CustomAudit extends Audit.

But, if I try to instead use an .mts file and write customAudit.mts in typescript, I get "Error: Cannot find module '.../customAudit.mts.js"

I believe if the compiled .mts compiled to a .mjs instead of a .mts.js this approach could work, as node would be able to run the .mjs. Hope this makes sense :)

@dgozman

@segevfiner
Copy link
Author

Additional issues:

  • Playwright fails to handle sub-path imports of packages that don't have exports in their package.json in ESM mode, it will require adding the .js extension. Try with import { lowlight } from lowlight/lib/all, in ESM mode.
  • Playwright fails to handle JSON imports without import assertions in ESM mode. Plenty of code uses those. And import assertions can be a problem to use, e.g. in a Vue SFC. Try import mermaidPackageJson from 'mermaid/package.json'; in ESM mode.

@nichita-pasecinic
Copy link

I have same issues in ESM mode:

could not import json files directly

TypeError: Module "path/config.json" needs an import attribute of type "json"

dayjs plugins does not work when importing without .js extension

import advancedFormat from 'dayjs/plugin/advancedFormat.js'; // <- .js need to be added
import duration from 'dayjs/plugin/duration.js';

If I decide to use ESM mode for entire project (e.g.: add "type": "module" to my package.json), how do I configure properly the playwright to work with it :/ ?

@alexandersorokin
Copy link
Contributor

alexandersorokin commented Dec 11, 2023

If I decide to use ESM mode for entire project (e.g.: add "type": "module" to my package.json), how do I configure properly the playwright to work with it :/ ?

You can use require to import commonjs code:

import { createRequire } from "module";
const require = createRequire(import.meta.url);

const advancedFormat = require('dayjs/plugin/advancedFormat'); 
const json= require('path/config.json'); 

For commonjs code import statements are interpreted as require anyway. In true ESM you can choose to use import or require explicitly.

@nichita-pasecinic
Copy link

You can use require to import commonjs code:

Then I get such error in client side code

Uncaught (in promise) Error: Module "module" has been externalized for browser compatibility. Cannot access "module.createRequire" in client code.

@coopbri
Copy link

coopbri commented Dec 28, 2023

This wouldn't be the ideal solution for everyone, but since the Bun runtime handles CJS/ESM resolution under the hood, I would imagine that integrating the Bun runtime to run Playwright tests would render this a non-issue for those that are willing to use it (instead of Node.js). However, that is easier said then done, especially as Playwright currently has Node.js version checks baked in. Relative issues from both communities (PW & Bun):

EDIT: looks like this is on the way! Jarred (Bun creator) created this PR 30min ago: #28875

@samhh
Copy link

samhh commented Jan 11, 2024

I'm not sure if this is expected behaviour, however I've observed the following. Maybe it'll be helpful for someone else.

I'm trying to migrate a monorepo to ESM. There are some packages which are expected to be consumed by a bundler rather than other packages. These aren't quite ESM-compliant and need to be compiled as TypeScript source by any consumers such as Playwright.

I can force Playwright to compile these packages via paths as per #14303, and alongside tsx I can have it accept their imports.

However, I would have expected "moduleResolution": "bundler" to work (#22169, #22887) without the need to involve tsx. Instead it seems to break the paths workaround.

@vhscom
Copy link

vhscom commented Feb 12, 2024

Using tsup to package up some vanilla test helpers and ran into issues consuming the bundled code with Playwright until I let CJS have the .js extension.

@tsirlucas
Copy link

is https://github.com/kolodny/safetest a possible solution to this issue? im still trying to figure things out but maybe we can make it work

@vincerubinetti
Copy link

vincerubinetti commented Mar 4, 2024

Playwright fails to handle JSON imports without import assertions in ESM mode.

This is becoming a catch-22 problem for me because of this issue in a prettier plugin. I need the type assertion so my Playwright e2e tests fail, but I then my lint/formatting tests fail.

Is there absolutely any way, without removing "type": "module" (which I need for Vite), to have Playwright be able to parse this?

Edit: Was able to work around this by just changing my Vite config file to .mjs extension, though not sure about the long term consequences of using module syntax just for Vite and not enforcing it everywhere.

@dorsharonfuse
Copy link

This has rendered our usage of Playwright's component testing unusable, since we're using lodash-es and as soon as some file imports that, we get an error since its an ESM-only lib, and we can't use type: module since we have a single package.json monorepo and that will create too much chaos for us ATM.

Is there nothing that can be done?
Is there any possible transform that at least transform all lodash-es imports to lodash or something?

@divmgl
Copy link

divmgl commented Mar 20, 2024

Can we get some answers for this? We're stuck with barebones Playwright tests because we can't actually execute any backend code in our E2E tests, which is often necessary to set up state for tests. I'd hate to go back to Cypress at this point.

Currently dealing with

TypeError: Module "file:///node_modules/.pnpm/@dqbd+tiktoken@1.0.7/node_modules/@dqbd/tiktoken/encoders/cl100k_base.json" needs an import attribute of type "json"

And this is after doing all kinds of ESM changes in our monorepo.

@nichita-pasecinic
Copy link

Currently dealing with

TypeError: Module "file:///node_modules/.pnpm/@dqbd+tiktoken@1.0.7/node_modules/@dqbd/tiktoken/encoders/cl100k_base.json" needs an import attribute of type "json"

@divmgl you can try:

import file from './file.json' assert { type: 'json' };

@divmgl
Copy link

divmgl commented Mar 21, 2024

Currently dealing with

TypeError: Module "file:///node_modules/.pnpm/@dqbd+tiktoken@1.0.7/node_modules/@dqbd/tiktoken/encoders/cl100k_base.json" needs an import attribute of type "json"

@divmgl you can try:

import file from './file.json' assert { type: 'json' };

? I don't have access to this library nor do I use it. Seems to be in the resolution path for one of our packages.

We've decided to stop using Playwright in this manner.

@ArCiGo
Copy link

ArCiGo commented Mar 22, 2024

Hi! I'm facing a similar error. We have two internal libraries (for API calls) that use ESNext. I updated my Playwright framework, and I'm not able to run my tests that use the code of these libraries.

I can't see in the UI Playwright mode those tests that use the logic of these two libraries. If I attempt to execute one of these tests through the terminal, I get an error complaining about one of the files that come from these libraries.

I attached the images of the existing configuration I use.

image

image

image

image

image

image

image

Through UI mode.-

image

Through terminal.-

image

I added the file extensions in my imports, and continues not working. I removed the "type": "module" from the package.json, but I got some weird(?) errors in two files I have in my project (and I don't have require-statements in my project).

image

image

Is there a solution for these problems?

@divmgl
Copy link

divmgl commented Mar 22, 2024

It looks there will soon be a patch incoming for Node.js that fixes the ESM/CJS interoperability. May be worth keeping a watch on this. nodejs/node#51977

@aaly00
Copy link

aaly00 commented Apr 2, 2024

This is causing problems for us as well with lodash-es

@olayway
Copy link

olayway commented Apr 15, 2024

Is there really no solid solution for this atm? 😭

@shortsyoungster
Copy link

I'm getting exactly the same behavior as @ArCiGo. Did you ever figure it out?

@piet-v
Copy link

piet-v commented Apr 26, 2024

@aaly00

For our internal project I wrote a custom patch for playwright using patch-package to add in support for a custom resolver in playwright.config.ts. Hopefully this functionality becomes built-in at some point in the future

Here's the patch file:

diff --git a/node_modules/playwright/lib/transform/transform.js b/node_modules/playwright/lib/transform/transform.js
index 8b30f78..09c464c 100644
--- a/node_modules/playwright/lib/transform/transform.js
+++ b/node_modules/playwright/lib/transform/transform.js
@@ -173,6 +173,7 @@ function calculateHash(content, filePath, isModule, pluginsPrologue, pluginsEpil
   const hash = _crypto.default.createHash('sha1').update(isModule ? 'esm' : 'no_esm').update(content).update(filePath).update(version).update(pluginsPrologue.map(p => p[0]).join(',')).update(pluginsEpilogue.map(p => p[0]).join(',')).digest('hex');
   return hash;
 }
+let customResolver = undefined;
 async function requireOrImport(file) {
   const revertBabelRequire = installTransform();
   const isModule = (0, _util.fileIsModule)(file);
@@ -185,6 +186,9 @@ async function requireOrImport(file) {
       const module = require.cache[file];
       if (module) collectCJSDependencies(module, depsCollector);
     }
+    if (file.includes("playwright.config.ts")) {
+      customResolver = result.default?.build?.resolver;
+    }
     return result;
   } finally {
     revertBabelRequire();
@@ -196,6 +200,9 @@ function installTransform() {
   const originalResolveFilename = _module.default._resolveFilename;
   function resolveFilename(specifier, parent, ...rest) {
     if (!reverted && parent) {
+      if (customResolver) {
+        specifier = customResolver(parent.filename, specifier);
+      }
       const resolved = resolveHook(parent.filename, specifier);
       if (resolved !== undefined) specifier = resolved;
     }
diff --git a/node_modules/playwright/types/test.d.ts b/node_modules/playwright/types/test.d.ts
index bbbcdb4..3bc2595 100644
--- a/node_modules/playwright/types/test.d.ts
+++ b/node_modules/playwright/types/test.d.ts
@@ -593,6 +593,7 @@ interface TestConfig {
      * test uses are listed here.
      */
     external?: Array<string>;
+    resolver: (filename: string, specifier: string) => string
   };
 
   /**

You can apply it using patch-package and then you can simply add a resolver function in your playwright.config.ts to override imports where needed:

build: {
        external: ['foo'],
        resolver: (filename, specifier) => {
            if (specifier === 'lodash-es') {
                return `lodash`;
            }

            return specifier;
        },
    },

if you want to mock certain files you can point to a mock file instead like:

        resolver: (filename, specifier) => {
            if (specifier === 'lodash-es') {
                return `${__dirname}/empty-mock.ts`;
            }

            return specifier;
        }

if you want to ignore certain files (in my case I want to ignore certain files from barrel files 'cause they import Angular down the line):

        resolver: (filename, specifier) => {
            if (specifier.includes('.service')) {
                return `${__dirname}/empty-mock.ts`;
            }

            return specifier;
        }

@tsirlucas
Copy link

has anyone tried with node 22? it seems you can require esm now

@PandharabaleAniketJohnDeere

With the current sorry state of the JavaScript ecosystem, an ongoing struggle is the CJS/ESM modules hell. Whereby, importing CJS from ESM, or ESM from CJS. is troublesome in plain Node, and only really works well in bundlers. Sadly, many things will force you to use CJS (Due to not supporting ESM), and many many modules are ESM only, or otherwise CJS with no compatibility for Node's ESM named import rules, or file extension/no directory imports requirements.

An example is this https://github.com/segevfiner/playwright-ct-vue-on-issue/blob/main/src/components/__tests__/esm-hell.pw.ts

This test imports a TypeScript file that imports lodash-es which is ESM only. This fails in Playwright without "type": "module" set as you can't require an ESM module. With "type": "module", you then hit a truck load of other issues with the poor state of Node's native ESM mode. All imports in the entire code base, including in dependencies, must be adjusted to include file extensions, which TypeScript doesn't seem to check for, except with "module/moduleResolution": "node16" but that mode is troublesome with bundlers. Further more, many CJS modules are not compatible with named imports from ESM as they don't define their exports in a statically analyzable way, this again results in aggravating failures when trying to import them in Node's ESM mode.

To handle this hellish nightmare, I found myself having to manually configure test runners to transform modules that have trouble importing in either CJS or ESM mode, depending on what the test runner uses, and to use tsx to run scripts as it transforms the modules using esbuild to the proper format before Node.

It would be very helpful if @playwright/test like Jest and Vitest, had a way to transform such problems modules too. When running in CJS mode, transform ESM modules to CJS, and vice versa when running in ESM mode, so that we are not limited by the annoying limitations of Node, which bundlers are not limited by, and can focus on actually writing code rather than fighting in the unholy CJS vs ESM purity war.

In Jest this is done by configuring a negative lookahead regexp in transformIgnorePatterns, and configuring your transform to perform the needed conversion, e.g. Babel's ESM to CJS.

In Vitest you sometimes need to configure deps.inline, including all modules that lead to such a problem module, as Vitest doesn't transform after an inner import/require of a non-transformed module. (It's not a require/loader hook like Jest ATM).

Is there any link to repo where you have manually configured test runners to transform modules that have trouble importing in either CJS or ESM mode?

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

No branches or pull requests