remix-run / remix

Build Better Websites. Create modern, resilient user experiences with web fundamentals.
https://remix.run
MIT License
28.47k stars 2.4k forks source link

SPA Mode: `remix reveal` generate entries files with unexpected content #8881

Closed okalil closed 4 months ago

okalil commented 4 months ago

Reproduction

  1. Create a new Remix app with SPA template npx create-remix@latest --template remix-run/remix/templates/spa
  2. Delete entry.server.tsx and entry.client.tsx files
  3. Run remix reveal

System Info

System:
    OS: Windows 10 10.0.19045
    CPU: (8) x64 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz
    Memory: 6.71 GB / 15.73 GB
  Binaries:
    Node: 18.18.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.10 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 9.8.1 - C:\Program Files\nodejs\npm.CMD
    pnpm: 8.6.0 - ~\AppData\Local\pnpm\pnpm.EXE
  Browsers:
    Edge: Chromium (121.0.2277.128)
    Internet Explorer: 11.0.19041.3636
  npmPackages:
    @remix-run/dev: ^2.7.2 => 2.7.2
    @remix-run/node: ^2.7.2 => 2.7.2
    @remix-run/react: ^2.7.2 => 2.7.2
    vite: ^5.1.0 => 5.1.4

Used Package Manager

npm

Expected Behavior

Since ssr is set to false in vite.config.ts, I would expect entry.server.tsx to contain the code that generates a static index.html file.

Actual Behavior

An entry.server.tsx for SSR is generated.

brophdawg11 commented 4 months ago

I would expect entry.server.tsx to contain the code that generates a static index.html file

That's exactly what it does πŸ˜‰. In order to generate the index.html file, Remix has pre-renders your / route during the build on the server - so it needs a server build and a server entry to handle that build-time Request. Once generated, the server build is removed. You are also free to remove the entry.server file if you have no need to customize it.

There's a note on this in the docs:

It's important to note that Remix SPA mode generates your index.html file by performing a "pre-render" of your root route on the server during the build. This means that while you're creating a SPA, you still have a "server build" and "server render" step, so you do need to be careful about using dependencies that reference client-only aspects such as document, window, localStorage, etc.

okalil commented 4 months ago

@brophdawg11 I think I didn't describe the problem well. I understand that Remix pre renders the index route to generate the static html; the issue is that if I delete that entry.server (that does the pre render), and run remix reveal to generate it again, I actually get the entry.server that is used for SSR apps.

brophdawg11 commented 4 months ago

Ah - ok yeah that's expected at the moment because remix reveal does not have access to the resolved remix plugin options inside the vite config so it can't determine if it's in SPA mode or not. Both of them work fine - the complex one and the simple one, so you can always go grab the simplified implementation from https://github.com/remix-run/remix/blob/main/packages/remix-dev/config/defaults/entry.server.spa.tsx if needed

kiliman commented 4 months ago

Yes, at the moment, all of the Remix CLI commands require the remix.config.js file to work. That's what I have in my remix-vite-template since I use remix-flat-routes and wanted to be able to run remix routes.

brophdawg11 commented 4 months ago

It hadn't occurred to me that this also impacts remix routes which is a bit of a bigger miss than remix reveal since the latter still reveals a working file (albeit a bit more complex one) - but remix routes is just incorrect. Because of the routes impact we're going to fix our CLI to be vite-config aware, which will allow reveal to be aware of the internals of the vite config. We're tracking the npx remix routes work in https://github.com/remix-run/remix/issues/8845 and I'm going to re-open this to get the npx remix reveal stuff fixed πŸ‘

markdalgleish commented 4 months ago

Fixed in https://github.com/remix-run/remix/pull/8916.

github-actions[bot] commented 4 months ago

πŸ€– Hello there,

We just published version 2.8.1-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!

github-actions[bot] commented 4 months ago

πŸ€– Hello there,

We just published version 2.8.1 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!