Skip to content

Conversation

@lucgonp
Copy link

@lucgonp lucgonp commented Nov 6, 2025

…ormance

  • Replace fs.readFileSync with async fs.readFile
  • Add proper error handling with debug logs
  • Improve performance by avoiding event loop blocking
  • Remove TODO comment about async fs methods

This follows Node.js best practices and improves DX with faster build times.

  • Closes

Additional details

Steps to test

How has the user experience changed?

PR Tasks


Note

Switches cypress.ts to non-blocking file reads, preloads the init script, and adds debug-backed error handling.

  • Vite Dev Server (npm/vite-dev-server/src/plugins/cypress.ts):
    • Async FS migration:
      • Replace synchronous fs.readFileSync and direct fs.promises.readFile with a shared promisified readFile helper.
      • Preload INIT_FILEPATH via loaderPromise and inject its contents in transformIndexHtml.
    • Error handling & logging:
      • Add loadInitFile() with debug logs and explicit error propagation.
    • HTML transformation:
      • Read index.html via async readFile and inject discovered <script> tags and the preloaded loader script.

Written by Cursor Bugbot for commit 8103f1c. This will update automatically on new commits. Configure here.

…ormance

- Replace fs.readFileSync with async fs.readFile
- Add proper error handling with debug logs
- Improve performance by avoiding event loop blocking
- Remove TODO comment about async fs methods

This follows Node.js best practices and improves DX with faster build times.
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@cypress-app-bot
Copy link
Collaborator

}

// Initialize loader promise
loaderPromise = loadInitFile()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Unhandled Promise Rejection in Plugin Loader

Potential unhandled promise rejection. The loaderPromise is initialized immediately when the plugin is created, but if the file read fails and transformIndexHtml is never called (e.g., in certain Vite configurations or error scenarios), the rejected promise will never be awaited, causing an unhandled promise rejection warning in Node.js. The old synchronous code would fail immediately at plugin creation, providing clearer error feedback. Consider adding a .catch() handler to the promise initialization or deferring the file read until it's actually needed in transformIndexHtml.

Fix in Cursor Fix in Web

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lucgonp We have an AI bot that reviews our PRs - sometimes the feedback is helpful, sometimes irrelevant - if you can respond accordingly here please.

Copy link
Member

@jennifer-shehane jennifer-shehane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lucgonp There are some errors in linting. You should be able to see them locally running: nx run @cypress/vite-dev-server:lint

Currently these errors are displaying:

$ eslint --ext .js,.ts,.json, .

/root/cypress/npm/vite-dev-server/src/plugins/cypress.ts
  54:7  error  Expected blank line before this statement  padding-line-between-statements
  55:7  error  Expected blank line before this statement  padding-line-between-statements

✖ 2 problems (2 errors, 0 warnings)
  2 errors and 0 warnings potentially fixable with the `--fix` option.

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants