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

fix(dev): optimizeDeps entries file path for vite #10258

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

sebastien-comeau
Copy link
Contributor

Pull Request Description:

Normalize Path for Vite Dependency Optimization on Windows

This PR addresses an issue with Vite's dependency optimization on Windows systems where incorrect path formats can lead to constant dependency reloading and performance degradation.

Problem:
On Windows, paths often use backslashes (\) as separators, while Vite expects forward slashes (/). This mismatch can cause Vite to incorrectly identify and optimize dependencies.

Example:

  • Incorrect Path: C:\data\projects\remix\app\entry.client.tsx
  • Correct Path: C:/data/projects/remix/app/entry.client.tsx

Solution:
This PR normalizes the paths using importViteEsmSync().normalizePath() to ensure they are compatible with Vite's dependency optimization.

Specific Changes:

  • Normalized Entry Point:
    - ctx.entryClientFilePath
    + importViteEsmSync().normalizePath(ctx.entryClientFilePath)
  • Optimized Route File Paths:
    - path.join(ctx.remixConfig.appDirectory, route.file)
    + resolveRelativeRouteFilePath(route, ctx.remixConfig)

Expected Benefits:

  • Improved Build Performance: Faster build times and smoother development experience.
  • Enhanced Stability: Prevents unexpected dependency reloads and other related issues.

Testing:

Please thoroughly test this change on Windows systems to verify that the issue is resolved and there are no unintended side effects.

Additional Considerations:

  • Other Potential Path Issues: Consider other areas where path normalization might be necessary.
  • Vite and Remix Versions: This fix has been tested with Vite v5.4.11 and Remix v2.14.0.

Please review and merge this PR if the changes are satisfactory.

Copy link

changeset-bot bot commented Nov 22, 2024

🦋 Changeset detected

Latest commit: cf57946

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 19 packages
Name Type
@remix-run/dev Patch
@remix-run/fs-routes Patch
@remix-run/route-config Patch
@remix-run/routes-option-adapter Patch
create-remix Patch
remix Patch
@remix-run/architect Patch
@remix-run/cloudflare Patch
@remix-run/cloudflare-pages Patch
@remix-run/cloudflare-workers Patch
@remix-run/css-bundle Patch
@remix-run/deno Patch
@remix-run/eslint-config Patch
@remix-run/express Patch
@remix-run/node Patch
@remix-run/react Patch
@remix-run/serve Patch
@remix-run/server-runtime Patch
@remix-run/testing Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@sebastien-comeau
Copy link
Contributor Author

The paths in Vite optimizeDeps.entries config option should be a fast-glob pattern. The path generated by Remix plugin on Windows have \ backslash and should be have /. fast-glob should be able to convert Windows path pattern automatically but Vite's using tinyglobby and it doesn't do that conversion. See the related tinyglobby issue.

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

Successfully merging this pull request may close these issues.

1 participant