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

Live reload doesn't work with a custom Express setup #3314

Closed
illright opened this issue May 27, 2022 · 10 comments
Closed

Live reload doesn't work with a custom Express setup #3314

illright opened this issue May 27, 2022 · 10 comments

Comments

@illright
Copy link
Contributor

What version of Remix are you using?

1.5.1

Steps to Reproduce

Create a new Remix app and configure a custom Express server as described in this example.

Run the development server and change some code. See the error in the browser console about not being able to connect to port 8002 and see no live reloading happen.

Terminate the development server and restart it, this time with an explicitly set environment variable, REMIX_DEV_SERVER_WS_PORT=8002. See no errors in the console and see live reloading happen upon code changes.

Expected Behavior

Live reloading defaults to port 8002 and works without explicitly setting the port, even for custom Express-based server setups.

Alternatively, I'd like this behaviour to be mentioned in the README of the repos that have a custom Express server because I had to figure this out by reading the source code.

Actual Behavior

Live reloading requires extra steps on every run for custom Express-based server setups.

@kiliman
Copy link
Collaborator

kiliman commented May 27, 2022

Yes. There's been a lot of tinkering with LiveReload and unfortunately while the intention was good (pick a random port for the web socket server), it is definitely broken in latest release.

Remix picks a random port, then sets REMIX_DEV_SERVER_WS_PORT. However, with the Express adapter, the Remix compiler/watcher is in a different process from the Express server. So that environment variable is not shared with Express.

NODE_ENV is the only environment variable that is applied at build time. The rest, like REMIX_DEV_SERVER_WS_PORT are at run time. So it's not until Remix renders the root layout from the initial GET, Remix doesn't know the port. Since it's undefined, it picks the default of 8002 which is different from remix watch.

Remix App Server works, since the watcher and web server are both started by the same process remix dev.

So I think the solution would be to update the Express template to set devServerPort in remix.config. Otherwise, you'd need a way to pass the port number across processes, which I think would over-complicate things.

@kiliman
Copy link
Collaborator

kiliman commented May 27, 2022

I did come up with a work-around if you need random ports with the Express adapter. The trick is to set the random port before calling run-p dev:*.

There's a CLI version of get-port which output the random port to stdout.
npm i -D get-port-cli

I wrote a script that sets the port, before calling remix/express (had to rename it to start:dev).

  "scripts": {
    "build": "remix build",
    "dev": "cross-env-shell NODE_ENV=development ./start-remix-dev",
    "start:dev": "run-p dev:*",
    "dev:node": "nodemon ./server.js --watch ./server.js",
    "dev:remix": "remix watch",
    "start": "cross-env NODE_ENV=production node ./server.js"
  },

start-remix-dev script

#!/usr/bin/env bash
REMIX_DEV_SERVER_WS_PORT=$(npx get-port)
echo "REMIX_DEV_SERVER_WS_PORT: $REMIX_DEV_SERVER_WS_PORT"
export REMIX_DEV_SERVER_WS_PORT
npm run start:dev

@illright
Copy link
Contributor Author

Thanks for your input! I'd rather stick to a hardcoded port in remix.config.js for now to keep complexity out of my repo and keep it cross-platform.

 /**
  * @type {import('@remix-run/dev').AppConfig}
  */
 module.exports = {
   cacheDirectory: "./node_modules/.cache/remix",
+  devServerPort: 8002,
   ignoredRouteFiles: ["**/.*", "**/*.css", "**/*.test.{js,jsx,ts,tsx}"],
   serverBuildDirectory: "./server/build",
 };

P.S. I wish the Remix config would accept promises, then one could simply call getPort from inside the config and avoid the hassle with scripts.

@mcansh
Copy link
Collaborator

mcansh commented Jun 6, 2022

i think this is caused by our LiveReload script being inline thus not being ran through esbuild, you should also be able to use <LiveReload port={Number(process.env.REMIX_DEV_SERVER_WS_PORT)} /> – looking into a better solution

@kiliman
Copy link
Collaborator

kiliman commented Jun 6, 2022

@mcansh The main reason is the Remix is setting REMIX_DEV_SERVER_WS_PORT with the random port, but since Express and other adapters like Netlify, run the web server portion in a separate process, it's not being read there, so it's using the default of 8002. So basically the random port thing only works on Remix App Server, where remix dev is managing both process and the variable is in scope.

The default templates for all the other adapters should be reverted to include the hard-coded devServerPort.

juliuxu added a commit to capraconsulting/nettsiden that referenced this issue Jun 13, 2022
@lukasa1993
Copy link

can it be some /dec/hot_reload endpoint and long pulling instead of separate port ? would also help with docker dev setups

@NeurAlch
Copy link

NeurAlch commented Jun 16, 2022

I just switched to using Remix with Node/Express and also have this issue. I'm using docker-compose / Docker and expected the 8002 default to work too.

Edit: I'm using express because I have a lot of already existing code that I want to use. I also can't have random ports since I need to configure a specific port on the docker-compose.yaml

@NeurAlch
Copy link

NeurAlch commented Jun 16, 2022

devServerPort: 8002,

Adding this solved the issue for me.

Edit: Did not think about adding this first to the config, since it was the default.

@tarngerine
Copy link

For others who stumble here: in my particular setup with docker, the port 8002 wasn't in the docker compose file as an allowed port, adding that to docker-compose.yml fixed LiveReload.

@pcattori
Copy link
Contributor

Next minor release should include the new dev server #5133 under the unstable_dev future flag, which wires up the live reload port at build-time automatically, so you don't have to set up any env vars for it at all.

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

No branches or pull requests

7 participants