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

vite/spa: npm run start using http-server does not support SPA routes #8623

Closed
ngbrown opened this issue Jan 29, 2024 · 6 comments
Closed

vite/spa: npm run start using http-server does not support SPA routes #8623

ngbrown opened this issue Jan 29, 2024 · 6 comments
Labels

Comments

@ngbrown
Copy link
Contributor

ngbrown commented Jan 29, 2024

Reproduction

Run this script (PowerShell):

npx create-remix@latest remix-spa-test -y --template remix-run/remix/templates/spa
cd .\remix-spa-test
echo "export default function Hello() { return <>Hello</>; }" | Out-File ./app/routes/hello.tsx -Encoding utf8
echo "import { Link } from `"@remix-run/react`";`n`nexport default function Index() { return <Link to=`"/hello`">Navigate</Link>; }" | Out-File ./app/routes/_index.tsx  -Encoding utf8
npm run build
npm run start

Open browser directly to http://127.0.0.1:8080/hello (change port if needed). Also try reloading the browser (F5) after navigating to the /hello route from the root (/) path.

System Info

System:
    OS: Windows 10 10.0.19045
    CPU: (24) x64 12th Gen Intel(R) Core(TM) i7-12850HX
    Memory: 31.75 GB / 63.67 GB
  Binaries:
    Node: 18.17.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.19 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 9.6.7 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Chromium (121.0.2277.83)
    Internet Explorer: 11.0.19041.3636
  npmPackages:
    @remix-run/dev: ^2.5.1 => 2.5.1
    @remix-run/node: ^2.5.1 => 2.5.1
    @remix-run/react: ^2.5.1 => 2.5.1
    vite: ^5.0.12 => 5.0.12

Used Package Manager

npm

Expected Behavior

When previewing a build, the server should support SPA fallback mode (serving multiple paths from a single index.html file) when using the SPA template. Use vite preview or sirv-cli --single as alternatives. (vite preview is powered by sirv).

No simple solutions that I found are especially production ready as they don't set the cache-control header for ./assets files separately from other files.

Actual Behavior

Navigating directly to a sub-route such as http://127.0.0.1:8080/hello fails with an HTTP ERROR 404.

Navigating to the page when starting at http://127.0.0.1:8080/ works correctly, but reloading once navigated to http://127.0.0.1:8080/hello also fails with an HTTP ERROR 404.

http-server does not support SPA mode. (it has been much discussed, but left unresolved, see http-party/http-server#772).

@brophdawg11
Copy link
Contributor

I'm a bit torn here 🤔

Yes - Remix SPA mode is built to allow you to load/hydrate any path on initial load. So in theory you're correct that when testing the prod build it should also allow that.

However, I don't love encouraging testing the prod build via something that does not match what you will actually use in prod (since vite preview is explicitly not production ready). I've not used vite preview enough to know if there's a surface area between vite preview and serving SPA mode in production some other way (i.e., via Express app.use('*', (req, res) => res.sendFile('index.html'))).

For example, would it be a potential footgun for folks to run vite preview to test their "prod" build and conclude they can load/hydrate non-/ routes - just to deploy to a CDN or github pages and find that it's broken?

It almost seems potentially safer to use something that aligns with the lowest common denominator of deploy modes (i.e., something like http-server which doesn't support this "SPA fallback" concept) and then let folks opt-into that via vite preview if that is their intended deployment strategy?

I dunno - maybe this is all a non-issue - if vite's decided that previewmode differences from production deploys isn't a concern, so can we?

@brophdawg11 brophdawg11 added package:dev feat:spa Issues related to SPA Mode labels Jan 30, 2024
@ngbrown
Copy link
Contributor Author

ngbrown commented Jan 30, 2024

@brophdawg11 Thanks for looking into this. I agree that this command isn't production ready, but neither is any other simple option without going back to a remix-serve type solution. From my issue:

No simple solutions that I found are especially production ready as they don't set the cache-control header for ./assets files separately from other files.

This is why my proposed solution is to drop the npm run start script and use npm run preview like Vite suggests to alleviate the possible confusion.

@ngbrown
Copy link
Contributor Author

ngbrown commented Jan 30, 2024

A patch is needed to get vite preview to work. It could be separated from the template change, but I think the template should still be modified to something that works with the single index.html SPA style output.

I mentioned a something closer to production-ready in the updated README.md:

You can serve this from any server of your choosing. The server should support SPA fallback. For a simple example, you could use [sirv-cli](https://www.npmjs.com/package/sirv-cli):
```shellscript
npx sirv-cli --single build/client/ --ignores "^/assets"
```

However, I wouldn't want someone to think that is the best solution either when vite preview probably works fine for what they are trying to do and it doesn't bring in any new dependencies.

Vite has nice documentation on deploying a static/SPA site and it might be worth linking to: https://vitejs.dev/guide/static-deploy.html

Edit: The Vite guide may be too general if it includes multi-page outputs from Vite. I don't think linking to it without checking is ideal. For example, as far as I am aware, GitHub pages doesn't directly support the path re-writing to /index.html so the work around is to copy index.html to 404.html, but the guide doesn't mention this.

@brophdawg11
Copy link
Contributor

ok yeah I think I can get on board with removing any opinions about how to deploy to production and just linking to Vite's docs there and main that apparent in the docs/template. I'll take a look over the PR when I have a moment 👍

Copy link
Contributor

🤖 Hello there,

We just published version 2.7.0-pre.0 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

Copy link
Contributor

🤖 Hello there,

We just published version 2.7.0 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

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

No branches or pull requests

2 participants