remix icon indicating copy to clipboard operation
remix copied to clipboard

feat(remix-dev): add `serverEntryFile` config option

Open kiliman opened this issue 3 years ago • 13 comments

Currently if a dev wants to extend the configuration of Remix App Server, they must eject from RAS to the Express adapter. This PR adds a new config option serverEntryFile that specifies the file that will be used by RAS instead of its default configuration. If the file is TypeScript, it will be transpiled automatically.

// remix.config.js
module.exports = {
  serverEntryFile: './server.ts'
}

Running npm run dev will launch Remix App Server and import the server file.

For production builds, the dev should update the start script to specify the server file for remix-serve. NOTE: If the server file is TypeScript, this must be transpiled manually during the build process.

"build": "remix build && npm run build:server",
"build:server": "esbuild --format=cjs --platform=node --outfile=build/server.js server.ts",
"start": "remix-serve build build/server.js",

The server file should export the following functions:

// REQUIRED: create the Express app and return it
export function createApp(
  buildPath: string,
  mode = "production",
  publicPath = "/build/",
  assetsBuildDirectory = "public/build/"
): Express

// OPTIONAL: create an HTTP/HTTPS server and return it
function createServer(app: Express, port: number): Server

// OPTIONAL: create a WebSocket server and return it
function createSocketServer(port: number): WebSocket.Server

Here's a sample server file that includes all the exports.

https://github.com/kiliman/remix-ras-server-example/blob/main/server.ts

kiliman avatar Sep 02 '22 15:09 kiliman

⚠️ No Changeset found

Latest commit: 45f916d4902d1d878525c7ad04bf0ca8ab058b7d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

changeset-bot[bot] avatar Sep 02 '22 15:09 changeset-bot[bot]

Aside from the readConfig test, I haven't added or modified any tests. I'm not really sure the best approach to testing this as it requires verifying remix dev, remix watch and remix-serve is processing the server file correctly.

Please advise.

kiliman avatar Sep 02 '22 15:09 kiliman

Nice! I liked what you mentioned here in #4107 (comment below)

[with this new change] ...you can specify your own server file for Remix App Server to customize your setup. This way Remix doesn't have to add a bunch of different configuration options. It gives you the flexibility of the Express adapter, but the DX of being able to simply run remix dev.


@remix-run/dev has a config.ts file which expose a server property, which one can use to define a server entry point according to the JSDoc comment right above it

I recall seeing @mcansh using that server property in this examples folder of his `remix-fastify repo a few days ago

@kiliman - is there an overlap with the new serverEntryFile config property and the existing server config property defined here?

cliffordfajardo avatar Sep 03 '22 02:09 cliffordfajardo

Yes there is a difference. The server option compiles your server file AND bundles it with your app build (including node_modules) into a single file.

The new option is just an extension of Remix App Server. I suppose I could have added an argument to remix dev instead.

kiliman avatar Sep 03 '22 16:09 kiliman

@kiliman - do you have any recommendations on how to test these changes out for folks who want to give your changes a test run?

  • do you have a patch file for patch-package
  • or if we clone your branch, after cloning your branch how would you recommend to proceed?

I'd love to provide feedback and trial out the API and report my findings back in this thread

cliffordfajardo avatar Sep 09 '22 12:09 cliffordfajardo

@cliffordfajardo I created a sample showing how to use a custom server file for RAS. It uses SSL and includes getLoadContext function.

https://github.com/kiliman/remix-ras-server-example

kiliman avatar Sep 10 '22 00:09 kiliman

It might be nice to show how consumers can use this to get the default @remix-run/serve server plus extra middleware:

// server/index.ts
import { createApp } from '@remix-run/serve'
import express from 'express'
import path from 'path'
import myCustomMiddleware from './myCustomMiddleware'

const app = express()
app.use(myCustomerMiddleware)
app.use(createApp(path.join(process.cwd(), 'build'));
export default app

That way, if folks follow the docs, they'll still be as close to @remix-run/serve as possible. They can always customize further if they want.

jamesarosen avatar Oct 20 '22 20:10 jamesarosen

@jamesarosen That's basically what this PR does. The issue is that the current Remix App Server doesn't expose the Express server, nor provide a way to override it.

This PR provides that extension point.

kiliman avatar Oct 20 '22 20:10 kiliman

That's basically what this PR does

I understood. I wasn't clear, but I was saying the docs should prefer importing the default server and extending it:

import { createApp } from '@remix-run/serve'
…
app.use(myCustomMiddleware)
app.use(createApp(…))

over replacing it altogether:

const { createRequestHandler } = require("@remix-run/express");
…
app.use(compression())
app.disable("x-powered-by")
app.use("/build", express.static("public/build", { immutable: true, maxAge: "1y" }))
app.use(express.static("public", { maxAge: "1h" }))
app.use(morgan("tiny"))
app.all(createRequestHandler(…))

That way, people who follow the docs will get the benefit of the default createApp with its caching, logging, and any new features the community adds over time.

jamesarosen avatar Oct 20 '22 21:10 jamesarosen

Ah, I think I see. So you're saying that in addition to my PR, make the default createApp an export so it can be used and extended in the server.ts file.

I agree. Sometimes you only want to make a simple change to the default, so may not want to write it from scratch (even if it is boilerplate).

My only concern is that Express app configuration is order dependent, so that my have unintended consequences, especially if the default is a black box.

kiliman avatar Oct 21 '22 14:10 kiliman

make the default createApp an export so it can be used and extended in the server.ts file. Exactly!

My only concern is that Express app configuration is order dependent, so that my have unintended consequences, especially if the default is a black box.

There are definitely pros and cons to both approaches. I don't think there's a clear winner. I spent a lot of time in the Ember community, which places a lot of value on preferring the framework, but offering a series of escalating escape hatches. In this case, that series would be

  1. use @remix-run/serve. A basic Remix-friendly HTTP server with TypeScript and live-reloading.
  2. serverEntryFile with createApp(). Add features to the basic server. Opt in to updates from the community.
  3. serverEntryFile with custom createRequestHandler(). You still get TypeScript and live-reloading, but opt out of updates from the community.
  4. server (I think; I'm not actually sure why someone would use this if serverEntryFile exists)
  5. replace @remix-run/serve with node myServer.js. Totally custom. No TypeScript or live-reloading unless you build it (e.g. with tsc and nodemon)

jamesarosen avatar Oct 21 '22 16:10 jamesarosen

  1. server is for hosts that don't allow you to run npm i for node_modules. This way you bundle everything and copy a single file to your host. Probably pretty rare considering how many providers there are.

kiliman avatar Oct 21 '22 18:10 kiliman

This change is exactly what I needed within Remix, I have lightly modified my express config but am now stuck using nodemon/tsc as described above. Using the framework in this case with an extended server.{t|j}s file would be such an benefit. Thanks for the hard work! Is there a timeline of when this would be merged into the core codebase?

danditomaso avatar Oct 27 '22 15:10 danditomaso

@kiliman I think we can close this as we now have entryServerFilePath config option since @mcansh's #4600?

MichaelDeBoey avatar Mar 09 '23 00:03 MichaelDeBoey

I think we can close this as we now have entryServerFilePath config option since @mcansh's #4600?

thats on the resolved config for entry.server - not the user's config

mcansh avatar Mar 09 '23 00:03 mcansh

Oh yeah I missed that it's not in the user's config, but the resolved config 🙈

MichaelDeBoey avatar Mar 09 '23 00:03 MichaelDeBoey

Yeah, I haven't been keeping this updated with the latest, as so many changes were happening with the new dev server.

kiliman avatar Mar 09 '23 00:03 kiliman

Hi @kiliman!

Are you still interested in getting this one merged?

If so, please rebase onto latest dev & resolve conflicts

MichaelDeBoey avatar May 01 '23 23:05 MichaelDeBoey

Hmm.. not sure. I think the new unstable_dev feature obviates the need for some of the items in this PR. I personally don't use it. I'll close it for now. I'll reconsider if somebody wants me to update it to the latest version.

kiliman avatar May 01 '23 23:05 kiliman