After Spec API TypeScript Code breaks the config.ts file - [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension ".ts"

GeorgeXCV commented 1 year ago

Current behavior

If I use the code from the snippet here for TypeScript it breaks the project:

TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension ".ts" for /Users/george/Documents/core-cypress/cypress.config.ts
    at new NodeError (node:internal/errors:372:5)
    at Object.getFileProtocolModuleFormat [as file:] (node:internal/modules/esm/get_format:76:11)
    at defaultGetFormat (node:internal/modules/esm/get_format:118:38)
    at defaultLoad (node:internal/modules/esm/load:21:20)
    at ESMLoader.load (node:internal/modules/esm/loader:407:26)
    at ESMLoader.moduleProvider (node:internal/modules/esm/loader:326:22)
    at new ModuleJob (node:internal/modules/esm/module_job:66:26)
    at ESMLoader.#createModuleJob (node:internal/modules/esm/loader:345:17)
    at ESMLoader.getModuleJob (node:internal/modules/esm/loader:304:34)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:385:24)
    at async importModuleDynamicallyWrapper (node:internal/vm/module:437:15)
    at async loadFile (/Users/george/Library/Caches/Cypress/10.3.0/Cypress.app/Contents/Resources/app/packages/server/lib/plugins/child/run_require_async_child.js:106:14)
    at async EventEmitter.<anonymous> (/Users/george/Library/Caches/Cypress/10.3.0/Cypress.app/Contents/Resources/app/packages/server/lib/plugins/child/run_require_async_child.js:116:32)

Even just this line breaks the project: const del = require('del')

Desired behavior

I can run my tests and the videos are deleted if test passed.

Test code to reproduce

/* eslint @typescript-eslint/no-var-requires: "off" */
import { defineConfig } from "cypress";
import { checkGmail, CheckGmailParam } from "../core-cypress/cypress/plugins/checkGmail";
import * as path from "path";
const del = require('del')

export default defineConfig({
  e2e: {
    async setupNodeEvents(on, config) {
      const version = config.env.version || 'development'
      const configFile = await import(path.join(
        config.projectRoot,
        'cypress/config',
        `${version}.json`
      ));
      config.projectId = "5jgpns"
      config.baseUrl = configFile.baseUrl
      config.env = configFile.env
      config.defaultCommandTimeout = 10000
      config.chromeWebSecurity = false
      on("task", {
        async checkGmail(args: CheckGmailParam) {
          return await checkGmail(args);
        },
      });
      on('after:spec', (spec, results) => {
        if (results && results.stats.failures === 0 && results.video) {
          // `del()` returns a promise, so it's important to return it to ensure
          // deleting the video is finished before moving on
          return del(results.video)
        }
      })
      return config
    },
    reporter: 'mochawesome'
  },
});

Cypress Version

10.1.0

Other

Current snippet also isn't 100% TypeScript/Eslint friendly because uses require instead of import and has unused parameter. Something like this should work right?

import { deleteAsync } from 'del'

on('after:spec', async (_spec, results) => {
        if (results && results.stats.failures === 0 && results.video) {
          // `del()` returns a promise, so it's important to return it to ensure
          // deleting the video is finished before moving on
          await deleteAsync(results.video)
        }
})

rockindahizzy commented 1 year ago

Right now there doesn't seem to be enough information to reproduce the problem on our end. Unless we receive a reliable reproduction, we'll eventually have to close this issue until we can reproduce it. This does not mean that your issue is not happening - it just means that we do not have a path to move forward.

Please provide a reproducible example of the issue you're encountering. Here are some tips for providing a Short, Self Contained, Correct, Example and our own Troubleshooting Cypress guide.

GeorgeXCV commented 1 year ago

Run any test with the above config file or just the module import in config file, super easy to reproduce.

rockindahizzy commented 1 year ago

Hi @GeorgeXCV, I was able to recreate and fix this issue in this repo: cypress-del-ts

The ticket for my project was adding "type": "module" to package.json.

I also made some slight changes in comparison to what the guide describes. import {deleteSync} from 'del'; and deleteSync(results.video) It seems to work, but it would be awesome if you could confirm the changes work for you.

GeorgeXCV commented 1 year ago

Hi @GeorgeXCV, I was able to recreate and fix this issue in this repo: cypress-del-ts

The ticket for my project was adding "type": "module" to package.json.

I also made some slight changes in comparison to what the guide describes. import {deleteSync} from 'del'; and deleteSync(results.video) It seems to work, but it would be awesome if you could confirm the changes work for you.

I get this error:

Your configFile is invalid: /Users/georgeashton/Documents/core/cypress.config.ts: —record

It threw an error when required, check the stack trace below:

ReferenceError: exports is not defined in ES module scope
    at file:///Users/georgeashton/Documents/core/cypress.config.ts:25:23
    at ModuleJob.run (node:internal/modules/esm/module_job:198:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:385:24)
    at async importModuleDynamicallyWrapper (node:internal/vm/module:437:15)
    at async loadFile (/Users/georgeashton/Library/Caches/Cypress/10.3.0/Cypress.app/Contents/Resources/app/packages/server/lib/plugins/child/run_require_async_child.js:106:14)
    at async EventEmitter.<anonymous> (/Users/georgeashton/Library/Caches/Cypress/10.3.0/Cypress.app/Contents/Resources/app/packages/server/lib/plugins/child/run_require_async_child.js:116:32)

Line 25 starts here:

on('after:spec', async (_spec, results) => {
        if (results && results.stats.failures === 0 && results.video) {
           // `del()` returns a promise, so it's important to return it to ensure
           // deleting the video is finished before moving on
           deleteSync(results.video)
        }
      })

Also why does Cypress recommend del module? Wouldn't built in fs module work?

import * as fs from "fs"

on('after:spec', async (_spec, results) => {
        if (results && results.stats.failures === 0 && results.video) {
            fs.unlink(results.video, (err) => {
               if (err) throw err
               return
          })
        }
      })

mike-plummer commented 1 year ago

Hi @GeorgeXCV - could you verify the contents of your tsconfig.json file at your project root? I was able to reproduce the "exports is not defined in ES module scope" issue you reported by removing this portion of the tsconfig.json from @rockindahizzy 's example:

"target": "es2016",
"module": "ES6",

In order to use imports in the config file the TS compiler will need to know how to handle module resolution which is provided by these keys

GeorgeXCV commented 1 year ago

Hi @GeorgeXCV - could you verify the contents of your tsconfig.json file at your project root? I was able to reproduce the "exports is not defined in ES module scope" issue you reported by removing this portion of the tsconfig.json from @rockindahizzy 's example:
"target": "es2016",
"module": "ES6",     
In order to use imports in the config file the TS compiler will need to know how to handle module resolution which is provided by these keys

Strange, I changed my tsconfig.json(previously using es5) but error persists:

{
    "compilerOptions": {
      "target": "es2016",
      "module": "ES6",
      "moduleResolution": "node",
      "esModuleInterop": true,
      "types": ["cypress", "node"]
    },
    "include": ["**/*.ts"]
  }

I am currently using fs.unlink instead using the snippet I posted here as I think that's better, doesn't require external module and don't need to reconfigure my TypeScript project for it.

mike-plummer commented 1 year ago

@GeorgeXCV , sorry to hear that didn't fix your issue. There must be some other configuration at play here that hasn't been communicated in this issue. Unfortunately, copying config files into the issue here only gives us a partial view that doesn't capture the entirety of your project context. To help us diagnose what's going on it would be very helpful to get a simple reproduction case as a Git repo. Here are some tips for providing a Short, Self Contained, Correct, Example and our own Troubleshooting Cypress guide.

nagash77 commented 1 year ago

Unfortunately we have to close this issue due to inactivity. Please comment if there is new information to provide concerning the original issue and we can reopen.

CSchulz commented 1 year ago

I have encountered the same issue with 10.3.1 and 10.9.0.

It seems to be related to the import itself. 10.9.0 gives a better error output but it doesn't help to solve the issue.

Your configFile is invalid: projects\e2e-tests\cypress.config.ts

It threw an error when required, check the stack trace below:

Error: Cannot find module '@e2e-commons/config'
Require stack:
- project\projects\hus\ph-e2e-vrk\cypress.config.ts
- Cypress\Cache\10.9.0\Cypress\resources\app\node_modules\@packages\server\lib\plugins\child\run_require_async_child.js
- Cypress\Cache\10.9.0\Cypress\resources\app\node_modules\@packages\server\lib\plugins\child\require_async_child.js
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:933:15)
    at Function.Module._resolveFilename (Cypress\Cache\10.9.0\Cypress\resources\app\node_modules\tsconfig-paths\lib\register.js:76:40)
    at Function.Module._resolveFilename.sharedData.moduleResolveFilenameHook.installedValue [as _resolveFilename] (Cypress\Cache\10.9.0\Cypress\resources\app\node_modules\@cspotcod
e\source-map-support\source-map-support.js:811:30)
    at Function.Module._load (node:internal/modules/cjs/loader:778:27)
    at Module.require (node:internal/modules/cjs/loader:1005:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at Object.<anonymous> (project\projects\hus\ph-e2e-vrk\cypress.config.ts:1:1)
    at Module._compile (node:internal/modules/cjs/loader:1105:14)
    at Module.m._compile (Cypress\Cache\10.9.0\Cypress\resources\app\node_modules\ts-node\src\index.ts:1618:23)
    at Module._extensions..js (node:internal/modules/cjs/loader:1159:10)
    at Object.require.extensions.<computed> [as .ts] (Cypress\Cache\10.9.0\Cypress\resources\app\node_modules\ts-node\src\index.ts:1621:12)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Module.require (node:internal/modules/cjs/loader:1005:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at loadFile (Cypress\Cache\10.9.0\Cypress\resources\app\node_modules\@packages\server\lib\plugins\child\run_require_async_child.js:89:14)

My paths of tsconfig.json seems to get ignored:

"paths": {
  "@e2e-commons": [
    "dist/e2e-commons",
    "node_modules/@e2e-commons"
  ],
  "@e2e-commons/config": [
    "dist/e2e-commons/config",
    "node_modules/@e2e-commons/config"
  ]
}

lmiller1990 commented 1 year ago

Can you include your full tsconfig.json

Also what is @e2e-commons/config? Can you share the package.json and whatever it is supposed to be pointing to (what's in dist/e2e-commons and node_modules/@e2e-commons/config? I didn't know you could point to multiple things in paths like this, how does it decide which one to use?)

I need more information to reproduce the issue.

CSchulz commented 1 year ago

I think I have found the issue in my setup. We have changed the build process from CJS to module builds. In our case the build of the package @e2e-commons/config also changed. You find CJS in the require stacktrace (see before).

@e2e-commons/config is our shared e2e test setup containing thinks like setupNodeEvents and commons config options.

Typescript takes the first path which can be resolved. More details can be found here, https://www.typescriptlang.org/docs/handbook/module-resolution.html#path-mapping. But it doesn't matter even if there is only one path included.

I have noticed that it works if I don't use the built package and build it on the fly.

The complete tsconfig.json:

{
    "compilerOptions": {
        "allowSyntheticDefaultImports": true,
        "baseUrl": "../../..",
        "declaration": false,
        "downlevelIteration": true,
        "esModuleInterop": true,
        "experimentalDecorators": true,
        "forceConsistentCasingInFileNames": true,
        "importHelpers": true,
        "lib": [
            "es2020",
            "dom"
        ],
        "module": "es2020",
        "moduleResolution": "node",
        "noFallthroughCasesInSwitch": true,
        "noPropertyAccessFromIndexSignature": false,
        "noImplicitReturns": true,
        "noUnusedLocals": true,
        "noUnusedParameters": true,
        "resolveJsonModule": true,
        "sourceMap": true,
        "strict": true,
        "strictPropertyInitialization": false,
        "target": "es2017",
        "typeRoots": [
            "project/node_modules/@types"
        ],
        "paths": {
            "@e2e-commons": [
                "dist/e2e-commons",
                "node_modules/@e2e-commons"
            ],
            "@e2e-commons/config": [
                "dist/e2e-commons/config",
                "node_modules/@e2e-commons/config"
            ]
        },
        "types": [
            "cypress",
            "node",
        ]
    },
    "include": [
        "**/*.ts"
    ]
}

The resulting package.json from the build looks like the following:

{
  "module": "../fesm2015/e2e-commons-config.mjs",
  "es2020": "../fesm2020/e2e-commons-config.mjs",
  "esm2020": "../esm2020/config/e2e-commons-config.mjs",
  "fesm2020": "../fesm2020/e2e-commons-config.mjs",
  "fesm2015": "../fesm2015/e2e-commons-config.mjs",
  "typings": "e2e-commons-config.d.ts",
  "sideEffects": false,
  "name": "@e2e-commons/config"
}

ghost commented 1 year ago

I ran into a similar issue yesterday and found a solution (although it may not be applicable to all cases). So, it's not documented anywhere, but looks like this error happens if your tsconfig.json file has compiler options that are not supported by your current Typescript version. In this case it just fails with this extension error and TS itself doesn't provide any hints to indicate that the config is not relevant for the current version

CSchulz commented 1 year ago

I have checked the bundled ressources of cypress, but didn't find any integrated typescript version. Also the changelog doesn't mention any typescript version. We are using typescript@4.6.4.

lmiller1990 commented 1 year ago

Sorry for the slow reply, I was out of office in October. We don't ship a TypeScript version - we use the one you've got installed (if there is one).

We have changed the build process from CJS to module builds I have noticed that it works if I don't use the built package and build it on the fly

Right, so you changed from CJS to ESM, and the built version is in ESM (JS extension) so it's trying to consume a ESM package using a CJS loader, I think?

Can you provide a minimal reproduction by any chance? It sounds like this is the problem. Most of the issues around ERR_UNKNOWN stem from some erroneous ESM/CJS interop.

Also what version of Node.js are you on? 16.16 and 16.17 handled the loaders a little differently (shot in the dark, but just throwing some ideas out).

CSchulz commented 1 year ago

I can try to setup a small reproduction repo.

I have tried 16.15.0, 16.17.1 and 16.18.1. All have the same error. Even an update to cypress 10.11 doesn't change anything.

iamsellek commented 1 year ago

Just wanted to report that I'm seeing this, as well! Here's my config file. If I get rid of the on function, the error goes away. Since this is the case, I'm wondering if it has something to do with the fact that this code is running in the node environment before Cypress loads all its stuff?

export default defineConfig({
  e2e: {
    setupNodeEvents(on, config) {
      on('after:run', () => {
        fetch(url, {
          method: 'POST',
          body,
        });
      });
    },
    supportFile: 'cypress/support/e2e.ts',
    experimentalSessionAndOrigin: true,
    chromeWebSecurity: false,
    defaultCommandTimeout: 6000,
    env: {
      ...process.env,
    },
  },
});

Edit: Strike that. It seems to be the use of fetch, which is imported, that's causing the issue.

CSchulz commented 1 year ago

How did you import the fetch?

iamsellek commented 1 year ago

@CSchulz import fetch from 'node-fetch'; I ended up taking another route with this, but node-fetch's current version is using ESModules, so that was probably the issue, I think?

lmiller1990 commented 1 year ago

I had a similar problem node-fetch, latest version is ESM only - you can use the older version (v2.x I think?) if your project isn't using type: module.

lmiller1990 commented 1 year ago

Is this still an issue or can we close it off? Is there a minimal repro I can pull?

CSchulz commented 1 year ago

But this would not fix the issue with ESM only libraries like we would have. I am trying to create a repro the next days.

lmiller1990 commented 1 year ago

Agreed, this is not perfectly solved. I'd like to see us move to esbuild eventually. It's faster and solves these problems fairly well. Until then, if you have a repro, I will do my best to fix it.

CSchulz commented 1 year ago

After some digging I can confirm the issue is similar to the node-fetch import with most recent version. As long an imported library is ESM, it is not supported yet.

lmiller1990 commented 1 year ago

If you are using type: module and your tsconfig is set to consume esnext, I think it should work? The thing we don't handle well is ESM only mixed with CJS.

karlhorky commented 1 year ago

@lmiller1990 considering that some projects cannot use "type": "module" in package.json (eg. frameworks which do not support ESM yet), would Cypress consider also supporting either A) a Cypress config file with .mts extension (cypress.config.mts) or B) an additional cypress open flag that would force ESM, both of which that would signal that the Cypress files should be interpreted as ESM modules?

The following scenario doesn't work, for example:

cypress.config.ts file in "type": "commonjs" package
import in cypress.config.ts of an ESM module (this should work with ts-node I guess?)

The error messages ranged from:

not being able to find the cypress.config.ts file at all, even though it was there
the typical TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension ".ts"
the also common Error [ERR_REQUIRE_ESM]: Must use import to load ES Module

Or any other way of forcing the ts-node in Cypress to use the ESM version, even if the "type": "module" is not specified in package.json.

I also tried a combination of the ts-node config options below, but these were also not helpful to configure the internal ts-node the way that it should be:

  "ts-node": {
    "compilerOptions": {
      "module": "esnext"
    }
    "esm": true
    "experimentalSpecifierResolution": "node",
    "moduleTypes": {
      "cypress.config.ts": "esm"
    }
  }

Lastly, I tried the "ESM dynamic import()" suggestions from a CommonJS TypeScript file consumed via ts-node here, which also did not help:

https://github.com/TypeStrong/ts-node/discussions/1290

To be clear, the use case here is:

cypress installed in same package as non-ESM lib package
cypress config imports ESM, and all test files are ESM
lib package needs to stay as CommonJS

The files do not have overlap above - all Cypress code will be ESM.

@lmiller1990 would you be open to supporting this use case somehow? Eg. suggestion A) a Cypress config file with .mts extension (cypress.config.mts) or B) an additional cypress open flag that would force ESM?

karlhorky commented 1 year ago

Until this is properly fixed, some workarounds for the problem of a CommonJS config file importing an ESM module:

Workaround 1

Transpile TypeScript code from imported ESM modules before running tests, either with esbuild or tsc.

Downside: extra build step and complexity

Workaround 2

Use the script below as your test runner - this edits the package.json file to have "type": "module" and removes it again afterwards:

// scripts/cy.ts
import { exec } from 'node:child_process';
import { readFileSync, writeFileSync } from 'node:fs';
import { dirname, resolve } from 'node:path';
import { fileURLToPath } from 'node:url';

const packageJsonPath = resolve(
  dirname(fileURLToPath(import.meta.url)),
  '..',
  'package.json',
);
const packageJson = JSON.parse(readFileSync(packageJsonPath, 'utf-8'));

writeFileSync(
  packageJsonPath,
  JSON.stringify({ type: 'module', ...packageJson }, null, 2),
);
console.log(`✅ Added "type": "module" to ${packageJsonPath}`);

process.on('SIGINT', () => {
  writeFileSync(packageJsonPath, JSON.stringify(packageJson, null, 2));
  console.log(`\n✅ Reverted ${packageJsonPath}`);
});

const cypressProcess = exec('cypress open --e2e --browser chrome');

cypressProcess.stdout?.pipe(process.stdout);
cypressProcess.stderr?.pipe(process.stderr);

Add to your package.json using tsm or tsx:

{
  "scripts": {
    "cy": "node --loader tsm scripts/cy.ts"
  },
  "devDependencies": {
    "tsm": "2.3.0"
  }
}

If you're using cypress-io/github-action, then you'll need to add an additional step to your GitHub Actions workflow beforehand to edit your package.json using yq (because this cy.ts script above will not be run by the action):

      # Add "type": "module" to package.json for proper Cypress ESM interop
      # https://github.com/cypress-io/cypress/issues/23141#issuecomment-1336532043
      - name: 'Add "type": "module" to package.json for Cypress ESM interop'
        run: yq --inplace --output-format=json '.type = "module"' package.json

If you need to revert it afterwards:

      # Revert Cypress ESM interop changes to package.json
      # https://github.com/cypress-io/cypress/issues/23141#issuecomment-1336532043
      - name: 'Revert Cypress ESM interop changes to package.json'
        run: yq --inplace --output-format=json 'del(.type)' package.json

Downside: extra build steps, script and complexity

lmiller1990 commented 1 year ago

@lmiller1990 would you be open to supporting this use case somehow? Eg. suggestion A) a Cypress config file with .mts extension (cypress.config.mts) or B) an additional cypress open flag that would force ESM?

I think we should just do whatever makes sense in the Node.js work - eg, if you are using CJS but want a specific file to use ESM, you can add a mjs file, and that'll work just fine.

I think there's some other edge cases we've missed - eg, you said

cypress.config.ts file in "type": "commonjs" package

Our entire code base is using the default (which is commonjs) and cypress.config.ts and it works fine, eg cypress.config.ts here, etc.

import in cypress.config.ts of an ESM module (this should work with ts-node I guess?)

This should work too - I tried it out just now - you need to set module in your tsconfig.json. I was able to import a module using .ts and ES module syntax (import and export).

The full suite of supported combinations is here (test projects are in system-tests/projects).

If you do have a use case that isn't working that should work, please share a minimal reproduction (eg, a Cypress project that doesn't start as expected) and I look into it. We should support all project configurations.

Edit right, I see your case:

cypress installed in same package as non-ESM lib package cypress config imports ESM, and all test files are ESM lib package needs to stay as CommonJS

Does this correctly reproduce the problem if you uncomment this line? https://github.com/lmiller1990/cypress-modules/blob/main/cypress.config.ts#L3 @karlhorky? If I set my module to es2015 in tsconfig.json, it does not like .js files using ESM syntax. es_module.js errors. es_module.ts works fine, though. So, I guess this problem is limted to modules installed from npm that are not ESM compatible? Is this correct?

karlhorky commented 1 year ago

eg, if you are using CJS but want a specific file to use ESM, you can add a mjs file, and that'll work just fine

Yeah, tried this as well, but unfortunately we're using TypeScript elsewhere, so that means an extra manual build step of transpiling those TypeScript files (Workaround 1 above), which is also not desirable.

Edit right, I see your case

Yeah, this was the one that I was struggling with. Some dragons there! Workarounds above are an option for now, but would be cool to have other options for switching to ESM mode from Cypress - other than "type": "module" in package.json, which won't work for some users.

I suggested a few different solutions to signal ESM modules usage - A) allowing the .mts config format and B) a new cypress open CLI flag (maybe --esm?)

karlhorky commented 1 year ago

Does this correctly reproduce the problem if you uncomment this line?

I don't think I meant that exactly, I forked and (I believe) reproduced the issue: https://github.com/lmiller1990/cypress-modules/pull/1/files

Made into a monorepo
Added another package with "type": "module" in package.json
Imported from a file in this new ESM package
Uncommented the import in the file in the PR above
Ran cypress open
Received error below

TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension ".ts" for /Users/k/p/cypress-modules/packages/cjs/cypress.config.ts
    at new NodeError (node:internal/errors:393:5)
    at Object.getFileProtocolModuleFormat [as file:] (node:internal/modules/esm/get_format:79:11)
    at defaultGetFormat (node:internal/modules/esm/get_format:121:38)
    at defaultLoad (node:internal/modules/esm/load:81:20)
    at nextLoad (node:internal/modules/esm/loader:163:28)
    at ESMLoader.load (node:internal/modules/esm/loader:605:26)
    at ESMLoader.moduleProvider (node:internal/modules/esm/loader:457:22)
    at new ModuleJob (node:internal/modules/esm/module_job:63:26)
    at ESMLoader.#createModuleJob (node:internal/modules/esm/loader:480:17)
    at ESMLoader.getModuleJob (node:internal/modules/esm/loader:434:34)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:530:24)
    at async importModuleDynamicallyWrapper (node:internal/vm/module:438:15)
    at async loadFile (/Users/k/Library/Caches/Cypress/11.2.0/Cypress.app/Contents/Resources/app/packages/server/lib/plugins/child/run_require_async_child.js:106:14)
    at async EventEmitter. (/Users/k/Library/Caches/Cypress/11.2.0/Cypress.app/Contents/Resources/app/packages/server/lib/plugins/child/run_require_async_child.js:116:32)

Screenshot 2022-12-05 at 01 15 43

lmiller1990 commented 1 year ago

Right, I see reproduction. Thanks for this.

I don't think we should add an --esm flag. It should be possible to configuring things using the actual tooling, eg ts-node and Node.js. We should support mts, though -- I didn't realize we didn't support that, but it looks like we do not check for it. Here's the files we do check for: https://github.com/cypress-io/cypress/blob/develop/packages/data-context/src/data/ProjectLifecycleManager.ts#L37-L42. Would that solve this problem?

Is there any way to execute that cypress.config.ts outside of Cypress, eg with ts-node? If that works, but Cypress doesn't, it's definitely a bug here. I don't think we can force ts-node to do something it doesn't support, but it's possible this will work with either a mts extension?

If ts-node cannot handle it independently, I don't think we can hack Cypress to do it, either. Let me play around a bit.

lmiller1990 commented 1 year ago

Just to clarify: is there any combination outside of Cypress where a CJS project can import an ESM file (using ts-node/typescript, since that's what Cypress is using)? Eg - can we execute that cypress.config.ts outside of the context of Cypress? If so, we should support this and we've got a bug - but if not, we'd be supporting an otherwise impossible use case.

My understanding is this isn't something Node.js generally supports - this module stuff is really confusing, though, so it's entire possible there's some combination of tooling that does support this.

Edit: I wonder if bundle-require would be a potential work around, until we either decide if this is a bug, or if it's intended behavior. That module uses esbuild to do the transformation.

Let me know if you have any ideas on the general question of whether this configuration of import .ts using esm into a CJS project is actually valid, and something we can support. We can definitely support mts, too, but that's separate from this - we should support a common format, even if it doesn't fix a specific bug. I'll file an issue.

karlhorky commented 1 year ago

I'm not 100% sure on that, but there was this part of my original comment above which may shed some light on that (I tried all 4 options, failed to get any of them to work with Cypress):

Lastly, I tried the "ESM dynamic import()" suggestions from a CommonJS TypeScript file consumed via ts-node here, which also did not help:

TypeStrong/ts-node#1290

lmiller1990 commented 1 year ago

Right, so basically they are recommending to try tsimportlib (or the helper). Neat hack, I didn't know CJS supported dynamic import (I wonder why that is; it's not CJS syntax?)

Isn't this something that would need to happen in user-land (eg, we couldn't do this in Cypress - we don't know about your module system, we just slurp it all up and run it using ts-node for CJS, or ts-node/esm for ESM)?

karlhorky commented 1 year ago

To be clear, I'm not sure whether it's supported or not - it's probably worth a bit of research to absolutely pin down what ts-node supports, considering it's a dependency of Cypress. I wouldn't be surprised if common scenarios do work out of the box with little ts-node configuration. Some ideas:

A) .mts files - maybe this just works B) can ts-node/esm always be used, also for CJS files? C) what interop options are there with ts-node? I posted some configuration options in my original comment above

Isn't this something that would need to happen in user-land

Hopefully not, this stuff is a nightmare to deal with, and leads to the feeling of Cypress being "broken", although we both know that this is a complicated interplay of module formats and dependencies. But just that we know - that doesn't help the users who are going through it.

I would suggest that ts-node module research should be done, and then the following solutions should be considered in order of priority:

Work out of the box if possible (can ts-node/esm always be used? eg. even with CJS files?). Cypress should contain all of the correct ts-node configuration options so that the user can avoid thinking about ESM interop - this is where the .mts option would come into play
If this is not possible for some reason, then offer a config option like --esm to force ts-node/esm only for Cypress files. Still not the best, because doesn't work out of the box, but better than userland solutions
Userland solution / workarounds / hacks

lmiller1990 commented 1 year ago

We can definitely look into this more, but quite a bit of research has been done, eg

A) .mts files - maybe this just works

Maybe - mts just turns into mjs, but this might help with consuming ts filers (like your example). I made an issue for that: https://github.com/cypress-io/cypress/issues/24962

B) can ts-node/esm always be used, also for CJS files?

I don't think this is supported. See here. "... Does not transform import ...". Then you are left with import - not require - so unless you've got mjs or type: module, Node.js won't know what to do.

C) what interop options are there with ts-node? I posted some configuration options in https://github.com/cypress-io/cypress/issues/23141#issuecomment-1336477281

From my understanding (could need more research) you basically pick one module system (cjs or esm) and if you need to interop, you do so by setting the extension (eg, if you are using cjs world, you use mjs, and if you are in esm world, you use cjs). I just tried this out, and it works - I have an mjs file that imports from a cjs file (which exports via module.exports).

this stuff is a nightmare to deal with, and leads to the feeling of Cypress being "broken"

This is the crux of the issue - Cypress feels broken, but it's generally incorrect configuration. Forcing --esm won't work, though - this is the same as adding type: module, isn't it?

Here's another explanation of the issue: https://gist.github.com/sindresorhus/a39789f98801d908bbc7ff3ecc99d99c.

If my understanding is correct, I think the best (and only?) action in this situation is a better error message? Cypress isn't a bundler, we can't do anything else when presented with an impossible module combination. Something like "You are trying to import a ES module into a CommonJS project. This isn't supported. See ____ for recommendations" may be the best option. WDYT?

karlhorky commented 1 year ago

Forcing --esm won't work, though - this is the same as adding type: module, isn't it?

No, a flag like this would signal the following, as I mentioned above:

package is unable to commit to a full-ESM module (potentially because of some dependency)
all Cypress files will be ESM

Taking your diagram:

What I'm suggesting is that "type": "module" should not be the only signal in the middle box - there should be other signals that indicate that this should use the ts-node/esm loader.

mts just turns into mjs

.mts could be another example of a "signal" like --esm that ts-node/esm should be used (switching to the bottom right branch in your diagram above).

If my understanding is correct, I think the best (and only?) action in this situation is a better error message?

I don't think that's the only option - I think there are at least 3 things to try out (try out using configuration to always transform to ESM, .mts extension and --esm flag)

B) can ts-node/esm always be used, also for CJS files?

I don't think this is supported. See here. "... Does not transform import ...". Then you are left with import - not require - so unless you've got mjs or type: module, Node.js won't know what to do.

We can definitely look into this more, but quite a bit of research has been done

Yeah as mentioned above, I think there's probably more configuration options to look into. ts-node has configuration options that allow treating a module as ESM:

https://replit.com/@karlhorky/ts-node-cjs-imports-esm#cjs/package.json

Screenshot 2022-12-05 at 10 45 14

tsconfig.json

{
  "ts-node": {
    "compilerOptions": {
      "module": "esnext"
    },
    "moduleTypes": {
      "index.ts": "esm"
    }
  }
}

Closer to my original suggestion of a cypress.config.mts config file, this moduleTypes configuration above is not needed if Cypress would support this config file format:

https://replit.com/@karlhorky/ts-node-mts-imports-esm#cjs/tsconfig.json

Screenshot 2022-12-05 at 11 00 53

karlhorky commented 1 year ago

I don't think this is supported. See here. "... Does not transform import ...". Then you are left with import - not require - so unless you've got mjs or type: module, Node.js won't know what to do.

Also, why is this important that require() is in the transpiled code? If the transpiled code is going to be run by Cypress, can't Cypress decide for itself that ESM is an acceptable module format for running code internally in Cypress? (since this is already working for code written in ESM format)

ts-node by itself has no problem running ESM syntax within a package with "type": "commonjs" in package.json. In the example below, the ESM-only import.meta.url feature is used:

https://replit.com/@karlhorky/ts-node-cjs-with-esnext#package.json

Screenshot 2022-12-05 at 10 55 22

index.ts

console.log('\nimport.meta.url (ESM-only):', import.meta.url);

tsconfig.json

{
  "ts-node": {
    "compilerOptions": {
      "module": "esnext"
    },
    "moduleTypes": {
      "index.ts": "esm"
    }
  }
}

lmiller1990 commented 1 year ago

Thanks for the detailed reply. Let me spend a bit of time reading and researching this.

My main concern with adding any kind of Cypress-specific flag/hint is that the same flag/hint should be possible with ts-node (and whatever toolchain the user has in their project). As long as said hint is present (eg, the mts in cypress.config.mts) Cypress will run all the code through ts-node with the configuration/hints presented by the user and work as it would without Cypress.

I think there are at least 3 things to try out ...

try out using configuration to always transform to ESM
.mts extension
esm flag

I think the first two are the most viable. I am not sure if we need to modify Cypress to do the first, do we? This should be doable in ts-node/userland. Or, we might need an additional hint here.

For mts, we should definitely do this, I created: https://github.com/cypress-io/cypress/issues/24962. We will look for mts, use ts-node/esm, and let ts-node do the rest.

As for a new command line flag, I think we need to consider carefully what it would actually do - would we ultimately we would just do some additional conditions here (similar to your first option) https://github.com/cypress-io/cypress/blob/e3435b6fba05480765769974d829e69cf1e42ca0/packages/data-context/src/data/ProjectConfigIpc.ts#L273-L310?

nagash77 commented 1 year ago

Unfortunately we have to close this issue due to inactivity. Please comment if there is new information to provide concerning the original issue and we can reopen.

karlhorky commented 1 year ago

Hm, seems a bit unusual to close an issue that has a detailed description of a problem that is still open... can't imagine the motivation for closing it. I suppose maybe the Cypress team discussed that it would clean up clutter? But there are more reasons to keep it open:

users run into this (very frustrating and confusing) problem, and keeping it open so that they can search for it is useful
closing as "unplanned" is actually not accurate - there are planned improvements, unresolved issues and unfinished investigations above
closing will lead to duplicate issues being posted, because an issue being "closed" can often mean that a new issue should be posted - which duplicates work (I understand that the post above mentions that new information should be posted here, but this will not always be observed or recognized by users who see a closed issue)

karlhorky commented 1 year ago

At the very least, this should stay open until the accepted resolutions which are described above (eg. cypress.config.mts) are merged + published in a release.

Then the authors / commenters in this issue (@GeorgeXCV, @CSchulz, @iamsellek, @nikolayeasygenerator, myself) should also be asked to comment whether the new release resolves their issue.

If they don't respond within a reasonable amount of time (2 weeks?) then closing seems reasonable at that point.

nagash77 commented 1 year ago

@karlhorky Sorry, this issue seems to have gotten incorrectly marked and not routed to the correct team and thus looked abandoned. I will reopen and route.

karlhorky commented 1 year ago

Thanks, no worries 👍

gilliardmacedo commented 1 year ago

I was at the same point of the issue creator. My config was working before, and after trying to include removal of sucessful test videos, it broke.

I repeated many steps of this thread, then I reverted all my work and I only rewrote the function. It worked:

on('after:spec', (spec, results: CypressCommandLine.RunResult) => {
        // delete the video if the spec passed and no tests retried
        if (results && results.video) {
          const failed = results.tests.some((test) => test.state === 'failed')
          if (!failed) {
            unlink(results.video, () => {
              console.log(`${results.video} deleted`)
            })
          }
        }
      })

Seems like the problem appears using lodash or del.

lmiller1990 commented 1 year ago

Seems like the problem appears using lodash or del.

del has a similar issue https://github.com/sindresorhus/del/issues/152

Could be unrelated to Cypress - can you post a minimal reproduction? Lodash should be working fine, I tried this with a ESM only project and did not encounter any issues.

I think there is many ways to encounter this issue, some might be an issue in Cypress, some with external modules.

adamalston commented 1 year ago

After I updated my config file using the changes from https://github.com/cypress-io/cypress-documentation/pull/5198, this issue was resolved for me. More specifically, the errors went away when I removed the return and substituted the del usage with its equivalent fs functionality.

lmiller1990 commented 1 year ago

Seems there are many issues that can manifest in this error. If anyone finds one, please post a new issue with a minimal reproduction, and we will take a look.

karlhorky commented 1 year ago

@lmiller1990 I don't think that this should be closed.

There is a long, detailed discussion between you and me in this issue which still should be investigated more.

I hope the Cypress team takes this issue seriously and decides to reopen, for the sake of your community.

karlhorky commented 1 year ago

At the very least, this should stay open until the accepted resolutions which are described above (eg. cypress.config.mts) are merged + published in a release.

Then the authors / commenters in this issue (@GeorgeXCV, @CSchulz, @iamsellek, @nikolayeasygenerator, myself) should also be asked to comment whether the new release resolves their issue.

lmiller1990 commented 1 year ago

Oh, I'm sorry, I think you are right - I didn't realize this was the issue with the mts discussion. I prematurely closed it. I agree, we should make sure we are in a good place with the whole ESM / CJS / TS loading.

We can track specific progress for cypress.config.mts here: https://github.com/cypress-io/cypress/issues/24962

cypress-io / cypress