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

Routing broken for preview with react-router-dom in 4.0.0-alpha.2 #10925

Closed
7 tasks done
fc opened this issue Nov 14, 2022 · 6 comments · Fixed by #11312
Closed
7 tasks done

Routing broken for preview with react-router-dom in 4.0.0-alpha.2 #10925

fc opened this issue Nov 14, 2022 · 6 comments · Fixed by #11312
Labels
p4-important Violate documented behavior or significantly improves performance (priority) regression The issue only appears after a new release

Comments

@fc
Copy link
Contributor

fc commented Nov 14, 2022

Describe the bug

When going to /some/route/somewhere using react-router-dom with preview the query path is not passed to the route leading to a 404 error.
In local dev it works fine.

git bisect indicates this PR is the cause:
#10475

Reproduction

https://stackblitz.com/edit/vitejs-vite-f9vj36?file=package.json,src%2FApp.jsx,vite.config.js&terminal=build,preview

Steps to reproduce

note: the StackBlitz link will run a build then run preview from the terminal query param. In dev the problem does not appear to happen.

Go to /test/about in the StackBlitz repro:
image

System Info

n/a

Used Package Manager

yarn

Logs

No response

Validations

@fc fc changed the title Routing broken with react-router-dom in 4.0.0-alpha.2 Routing broken for prod builds with react-router-dom in 4.0.0-alpha.2 Nov 14, 2022
@fc fc changed the title Routing broken for prod builds with react-router-dom in 4.0.0-alpha.2 Routing broken for prod builds with react-router-dom in 4.0.0-alpha.[012] Nov 14, 2022
@fc fc changed the title Routing broken for prod builds with react-router-dom in 4.0.0-alpha.[012] Routing broken for prod builds with react-router-dom in 4.0.0-alpha.2 Nov 14, 2022
@fc fc changed the title Routing broken for prod builds with react-router-dom in 4.0.0-alpha.2 Routing broken for preview (prod?) builds with react-router-dom in 4.0.0-alpha.2 Nov 14, 2022
@fc fc changed the title Routing broken for preview (prod?) builds with react-router-dom in 4.0.0-alpha.2 Routing broken for preview (and prod builds?) with react-router-dom in 4.0.0-alpha.2 Nov 14, 2022
@fc fc changed the title Routing broken for preview (and prod builds?) with react-router-dom in 4.0.0-alpha.2 Routing broken for preview with react-router-dom in 4.0.0-alpha.2 Nov 14, 2022
@fc
Copy link
Contributor Author

fc commented Nov 15, 2022

Some notes:

  • this block looks like the cause
  • when config.appType === 'spa' is true then then the single option of sirv is enabled. Docs - "When true, the directory's index page (default index.html) will be sent if the request asset does not exist."
  • so, I think we would want to mimic the behavior of sirv for spa apps and to fallback to routing to index.html when shouldServe is false.

@ElMassimo
Copy link
Contributor

ElMassimo commented Dec 9, 2022

Testing Vite 4 with îles.

An unintended effect of #10475 is that sirv will only receive requests to /existing if there's an existing/index.html file, while prior to the change sirv was able to resolve /existing to a existing.html file.

This is important to be able to locally preview sites that will be deployed using cleanUrls in Vercel, or in Netlify.

Ideally, frameworks like îles should be able to override shouldServe, as it limits the usefulness of the preview command.

@bluwy bluwy added p4-important Violate documented behavior or significantly improves performance (priority) regression The issue only appears after a new release and removed pending triage labels Dec 9, 2022
@mickdewald
Copy link

mickdewald commented Dec 10, 2022

I can confirm this error in vite 4.0.0 (stable) as well.
In dev everything works fine, but as soon as I run yarn preview refreshing a nested route it breaks, e.g.
http://localhost:3000/user
but navigating from http://localhost:3000/ to http://localhost:3000/user via link works fine

@sapphi-red sapphi-red mentioned this issue Dec 11, 2022
9 tasks
@mzaien
Copy link

mzaien commented Dec 11, 2022

  • of sirv is enabl

I can confirm this too, everything works fine with v3

I tried to run the build folder with server and it works correctly

@gbyesiltas
Copy link

I have the same issue on a Vue 3 project

@mickdewald
Copy link

I just installed v4.0.1 and my problem has been solved.
Awesome job guys and thank you!

I can confirm this error in vite 4.0.0 (stable) as well. In dev everything works fine, but as soon as I run yarn preview refreshing a nested route it breaks, e.g. http://localhost:3000/user but navigating from http://localhost:3000/ to http://localhost:3000/user via link works fine

@github-actions github-actions bot locked and limited conversation to collaborators Dec 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
p4-important Violate documented behavior or significantly improves performance (priority) regression The issue only appears after a new release
Projects
None yet
6 participants