Preprocessor can fail silently

[MetRonnie]

I am using vite-plugin-eslint. If I have an eslint error in a spec file, when I run Cypress, there is no indication of the error and Cypress keeps running an outdated spec no matter how many times I re-run the test. If I turn off the particular lint rule in eslintrc.js, then re-run the test it finally runs the up-to-date spec.

This is a very nasty day-wasting type of problem to encounter.

I don't know if it's a problem with cypress-vite or Cypress in general.

[MetRonnie]

OK I've found what should be an easy reproduceable example: just have a bad import in your spec file.

import something from 'non/exist'

No error, nothing to indicate anything is wrong unless you pay attention and notice that any subsequent changes to the spec file do not update the spec that Cypress actually runs.

[mammadataei]

I'm not sure how an Eslint plugin could stop the compilation of spec files. Are you using any Vite plugin that can fail the compilation when there is an Eslint error?

You can also try to run in debug mode to get more details from cypress-vite.

[MetRonnie]

Woops I meant vite-plugin-eslint. In any case that doesn't matter as I mentioned, a bad import can also cause a silent failure to transform the spec file.

[MetRonnie]

I tried DEBUG=cypress:* cypress open --e2e 2> cy.log and then grepped through the log

2023-03-31T09:45:26.376Z cypress:lifecycle:child:RunPlugins:54909 execute plugin event: file:preprocessor ({ invocationId: 'inv4', eventId: 4 })
2023-03-31T09:45:26.376Z cypress:lifecycle:child:RunPlugins:54909 execute plugin event: file:preprocessor ({ invocationId: 'inv5', eventId: 4 })
{
  code: 'ERROR',
  error: [Error: Could not load <THE_BAD_IMPORT_PATH> (imported by tests/e2e/specs/thing.cy.js): ENOENT: no such file or directory, open '<THE_BAD_IMPORT_PATH>'] {
    errno: -2,
    code: 'PLUGIN_ERROR',
    syscall: 'open',
    path: '<THE_BAD_IMPORT_PATH>',
    pluginCode: 'ENOENT',
    plugin: 'vite:load-fallback',
    hook: 'load',
    watchFiles: [
      '~/project/tests/e2e/specs/thing.cy.js',
      '<THE_BAD_IMPORT_PATH>'
    ]
  },
  result: null
}

[mammadataei]

Cypress-vite has no control over it since it is the Vite that is stopping the compilation. I don't know if there is a way to catch and notify the cypress about the error message.

For now, I think the simplest solution is to opt out the eslint plugin conditionally in Vite configuration for spec files:

export default defineConfig({
  plugins: [
    react(),
    tsconfigPaths(),
    // ...

    ...(process.env.CYPRESS ? [eslint()] : []), // Cypress automatically sets the `CYPRESS` env variable.
  ],
})

[MetRonnie]

Thanks for the process.env.CYPRESS trick, looks like that'll come in handy for other things too...

[MetRonnie]

...Vite that is stopping the compilation. I don't know if there is a way to catch and notify the cypress about the error message.

I had a bit of a play with vite.build(). There is an error being thrown for the bad import path which I am able to catch with await vite.build({ ... }).catch((e) => { ... }). ~So I'm guessing the problem is the error is being caught somewhere. I'll open an issue in Cypress~ Wait, actually no error is thrown by

https://github.com/mammadataei/cypress-vite/blob/22c8d9307c1b16fb975b83339e0838adb3cdadc6/src/index.ts#L66-L69

even though one is thrown when I run await vite.build() in the node REPL

Ah, now we're getting somewhere. The difference was I was setting watch: null in the config I was testing out in the REPL. If I set watch: null at

https://github.com/mammadataei/cypress-vite/blob/22c8d9307c1b16fb975b83339e0838adb3cdadc6/src/index.ts#L53

then vite.build() does throw.

[MetRonnie]

It looks to me like #36 should have fixed this?

Ok this is weird. I'm not very experienced with TypeScript but I think something when wrong when transpiling at v1.3.1?

Take a look at https://github.com/mammadataei/cypress-vite/blob/22c8d9307c1b16fb975b83339e0838adb3cdadc6/src/index.ts#L63-L95

and compare to https://www.npmjs.com/package/cypress-vite/v/1.3.1?activeTab=code index.cjs

It's like the changes from #36 weren't included?

[MetRonnie]

Ok, regardless it looks like #36 in of itself doesn't fix this issue though.

I did a silly amount of debugging trying to figure out this issue, turns out I don't think Vite/Rollup's built-in watch API can really be used for this purpose.

The preprocessor function has to return a Promise that resolves to the output path (if build was successful) or rejects with an Error (if build failed). The problem is that the docs say (emphasis mine):

This callback function can and will be called multiple times with the same filePath.

The callback function is called any time a file is requested by the browser. This happens on each run of the tests.

Make sure not to start a new watcher each time it is called. Instead, cache the watcher and, on subsequent calls, return a promise that resolves when the latest version of the file has been processed.

In your code, if the cache is hit for a particular spec file, the preprocessor resolves with the output path even if Vite had a build error https://github.com/mammadataei/cypress-vite/blob/22c8d9307c1b16fb975b83339e0838adb3cdadc6/src/index.ts#L31-L34

This is why it can fail silently.

Maybe I'm not good enough, but I tried to refactor your code to try and cache a Promise or a watcher instead of just the file path but could simply not get it to work. I think the rollup.watch() API is just too incompatible with how Cypress wants a preprocessor to function.

---

The good news is that it looks like it's possible to fix this and simplify the preprocessor, by using a chokidar watcher (actually Vite/Rollup uses chokidar behind the scenes).

All the preprocessor needs to do is (in reverse order actually):

• Call vite.build() with build.watch = null in the config
• Watch the file, and when it changes, do file.emit('rerun'), which will re-run the preprocessor
• Cache the watcher so that it or its event listener are not added on subsequent preprocesses

I can have open a PR if this sounds promising to you.

Alternative to chokidar is to just use node's fs.watch().

Sorry about the brain dump, I've become a little obsessed with this problem!

[jsakas]

It looks to me like #36 should have fixed this?

@mammadataei - I believe @MetRonnie is correct. The PR we merged in #36 is not included in v1.3.1 on npm. I think we need to run the build command and publish a new version ensuring the new dist is published.

You can see the dist for v1.3.1 here: https://www.npmjs.com/package/cypress-vite?activeTab=code

There is no return new Promise(...) which is the fix for a lot of the issues we are experiencing when there are larger bundles that need to be compiled.

[MetRonnie]

@jsakas However it doesn't fix the bug I'm reporting here. I can have a PR ready after Easter, essentially the code looks like this:

https://github.com/MetRonnie/cylc-ui/blob/e9046c7c380f7a1c17e761eb32fcd293a5d5efde/tests/e2e/support/vite-preprocessor.js

There is no need for the new Promise() constructor with this approach as it is just returning/throwing from an async function, which is equivalent to resolving/rejecting a promise

[mammadataei]

It looks to me like #36 should have fixed this?

@mammadataei - I believe @MetRonnie is correct. The PR we merged in #36 is not included in v1.3.1 on npm. I think we need to run the build command and publish a new version ensuring the new dist is published.

You can see the dist for v1.3.1 here: https://www.npmjs.com/package/cypress-vite?activeTab=code

There is no return new Promise(...) which is the fix for a lot of the issues we are experiencing when there are larger bundles that need to be compiled.

Apparently, something went wrong during the release of v1.3.1. I just release a patch. Should be fixed in v1.3.2.

[e1uone]

Thanks @MetRonnie you saved me long hours of debugging, simply because I suddenly removed eslint-plugin-cypress from the dev deps during the migration from webpack to vite, my config file was broken which led to this issue