feat(server file): add createServer (#9845)

This PR brings the server file out of experimental by implementing a
`createServer` function. This is a continuation of the work started in
#8119.

This API was designed in response to the feedback to #8119, which gave
users as much control as possible by more or less ejecting the code in
api-server. This resulted in users managing lot of code that really
wasn't their concern. In general it didn't feel like the Redwood way.
The new API still gives users control over how the server starts but
encapsulates the low-level details.

I've tried to make this PR as complete as possible. I feel like it's
reached that state, but there's still a few things I'd like to do. In
general I'd like to deduplicate all the repeated server code.

- [x] bring the server file out of experimental
- [x] type the `start` function
- [x] figure out how to handle the graphql function 
- [x] double check that `yarn rw dev` works well (namely, the watching)
- [x] double check that you can pass CLI args in dev and serve
- [x] the `yarn rw serve` command needs start two processes instead of
one with the server file
- [x] double check that env vars are being loaded
- [x] right now this is imported from `@redwoodojs/api-server`. long
term i don't think this is the best place for it

---------

Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
Co-authored-by: Josh GM Walker <56300765+Josh-Walker-GM@users.noreply.github.com>

Author: 3 people

packages/api-server/ambient.d.ts

@@ -0,0 +1 @@
+declare module 'dotenv-defaults'

packages/api-server/dist.test.ts

@@ -72,6 +72,7 @@ describe('dist', () => {
             "type": "string",
           },
         },
+        "createServer": [Function],
         "webCliOptions": {
           "apiHost": {
             "alias": "api-host",

packages/api-server/package.json

@@ -61,7 +61,16 @@
     "@types/yargs": "17.0.32",
     "aws-lambda": "1.0.7",
     "jest": "29.7.0",
+    "pino-abstract-transport": "1.1.0",
     "typescript": "5.3.3"
   },
+  "peerDependencies": {
+    "@redwoodjs/graphql-server": "6.0.7"
+  },
+  "peerDependenciesMeta": {
+    "@redwoodjs/graphql-server": {
+      "optional": true
+    }
+  },
   "gitHead": "3905ed045508b861b495f8d5630d76c7a157d8f1"
 }

packages/api-server/src/__tests__/createServer.test.ts

@@ -0,0 +1,330 @@
+import path from 'path'
+
+import pino from 'pino'
+import build from 'pino-abstract-transport'
+
+import { getConfig } from '@redwoodjs/project-config'
+
+import {
+  createServer,
+  resolveOptions,
+  DEFAULT_CREATE_SERVER_OPTIONS,
+} from '../createServer'
+
+// Set up RWJS_CWD.
+let original_RWJS_CWD
+
+beforeAll(() => {
+  original_RWJS_CWD = process.env.RWJS_CWD
+  process.env.RWJS_CWD = path.join(__dirname, './fixtures/redwood-app')
+})
+
+afterAll(() => {
+  process.env.RWJS_CWD = original_RWJS_CWD
+})
+
+describe('createServer', () => {
+  // Create a server for most tests. Some that test initialization create their own
+  let server
+
+  beforeAll(async () => {
+    server = await createServer()
+  })
+
+  afterAll(async () => {
+    await server?.close()
+  })
+
+  it('serves functions', async () => {
+    const res = await server.inject({
+      method: 'GET',
+      url: '/hello',
+    })
+
+    expect(res.json()).toEqual({ data: 'hello function' })
+  })
+
+  describe('warnings', () => {
+    let consoleWarnSpy
+
+    beforeAll(() => {
+      consoleWarnSpy = jest.spyOn(console, 'warn')
+    })
+
+    afterAll(() => {
+      consoleWarnSpy.mockRestore()
+    })
+
+    it('warns about server.config.js', async () => {
+      await createServer()
+
+      expect(consoleWarnSpy.mock.calls[0][0]).toMatchInlineSnapshot(`
+      "
+      Ignoring \`config\` and \`configureServer\` in api/server.config.js.
+      Migrate them to api/src/server.{ts,js}:
+      
+      \`\`\`js title="api/src/server.{ts,js}"
+      // Pass your config to \`createServer\`
+      const server = createServer({
+        fastifyServerOptions: myFastifyConfig
+      })
+      
+      // Then inline your \`configureFastify\` logic:
+      server.register(myFastifyPlugin)
+      \`\`\`
+      "
+    `)
+    })
+  })
+
+  it('`apiRootPath` prefixes all routes', async () => {
+    const server = await createServer({ apiRootPath: '/api' })
+
+    const res = await server.inject({
+      method: 'GET',
+      url: '/api/hello',
+    })
+
+    expect(res.json()).toEqual({ data: 'hello function' })
+
+    await server.close()
+  })
+
+  // We use `console.log` and `.warn` to output some things.
+  // Meanwhile, the server gets a logger that may not output to the same place.
+  // The server's logger also seems to output things out of order.
+  //
+  // This should be fixed so that all logs go to the same place
+  describe('logs', () => {
+    let consoleLogSpy
+    let consoleWarnSpy
+
+    beforeAll(() => {
+      consoleLogSpy = jest.spyOn(console, 'log').mockImplementation()
+      consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation()
+    })
+
+    afterAll(() => {
+      consoleLogSpy.mockRestore()
+      consoleWarnSpy.mockRestore()
+    })
+
+    it("doesn't handle logs consistently", async () => {
+      // Here we create a logger that outputs to an array.
+      const loggerLogs: string[] = []
+      const stream = build(async (source) => {
+        for await (const obj of source) {
+          loggerLogs.push(obj)
+        }
+      })
+      const logger = pino(stream)
+
+      // Generate some logs.
+      const server = await createServer({ logger })
+      const res = await server.inject({
+        method: 'GET',
+        url: '/hello',
+      })
+      expect(res.json()).toEqual({ data: 'hello function' })
+      await server.listen({ port: 8910 })
+      await server.close()
+
+      // We expect console log to be called with `withFunctions` logs.
+      expect(consoleLogSpy.mock.calls[0][0]).toMatch(
+        /Importing Server Functions/
+      )
+
+      const lastCallIndex = consoleLogSpy.mock.calls.length - 1
+
+      expect(consoleLogSpy.mock.calls[lastCallIndex][0]).toMatch(/Listening on/)
+
+      // `console.warn` will be used if there's a `server.config.js` file.
+      expect(consoleWarnSpy.mock.calls[0][0]).toMatchInlineSnapshot(`
+      "
+      Ignoring \`config\` and \`configureServer\` in api/server.config.js.
+      Migrate them to api/src/server.{ts,js}:
+      
+      \`\`\`js title="api/src/server.{ts,js}"
+      // Pass your config to \`createServer\`
+      const server = createServer({
+        fastifyServerOptions: myFastifyConfig
+      })
+      
+      // Then inline your \`configureFastify\` logic:
+      server.register(myFastifyPlugin)
+      \`\`\`
+      "
+    `)
+
+      // Finally, the logger. Notice how the request/response logs come before the "server is listening..." logs.
+      expect(loggerLogs[0]).toMatchObject({
+        reqId: 'req-1',
+        level: 30,
+        msg: 'incoming request',
+        req: {
+          hostname: 'localhost:80',
+          method: 'GET',
+          remoteAddress: '127.0.0.1',
+          url: '/hello',
+        },
+      })
+      expect(loggerLogs[1]).toMatchObject({
+        reqId: 'req-1',
+        level: 30,
+        msg: 'request completed',
+        res: {
+          statusCode: 200,
+        },
+      })
+
+      expect(loggerLogs[2]).toMatchObject({
+        level: 30,
+        msg: 'Server listening at http://[::1]:8910',
+      })
+      expect(loggerLogs[3]).toMatchObject({
+        level: 30,
+        msg: 'Server listening at http://127.0.0.1:8910',
+      })
+    })
+  })
+
+  describe('`server.start`', () => {
+    it('starts the server using [api].port in redwood.toml if none is specified', async () => {
+      const server = await createServer()
+      await server.start()
+
+      const address = server.server.address()
+
+      if (!address || typeof address === 'string') {
+        throw new Error('No address or address is a string')
+      }
+
+      expect(address.port).toBe(getConfig().api.port)
+
+      await server.close()
+    })
+
+    it('the `REDWOOD_API_PORT` env var takes precedence over [api].port', async () => {
+      process.env.REDWOOD_API_PORT = '8920'
+
+      const server = await createServer()
+      await server.start()
+
+      const address = server.server.address()
+
+      if (!address || typeof address === 'string') {
+        throw new Error('No address or address is a string')
+      }
+
+      expect(address.port).toBe(+process.env.REDWOOD_API_PORT)
+
+      await server.close()
+
+      delete process.env.REDWOOD_API_PORT
+    })
+  })
+})
+
+describe('resolveOptions', () => {
+  it('nothing passed', () => {
+    const resolvedOptions = resolveOptions()
+
+    expect(resolvedOptions).toEqual({
+      apiRootPath: DEFAULT_CREATE_SERVER_OPTIONS.apiRootPath,
+      fastifyServerOptions: {
+        requestTimeout:
+          DEFAULT_CREATE_SERVER_OPTIONS.fastifyServerOptions.requestTimeout,
+        logger: DEFAULT_CREATE_SERVER_OPTIONS.logger,
+      },
+      port: 8911,
+    })
+  })
+
+  it('ensures `apiRootPath` has slashes', () => {
+    const expected = '/v1/'
+
+    expect(
+      resolveOptions({
+        apiRootPath: 'v1',
+      }).apiRootPath
+    ).toEqual(expected)
+
+    expect(
+      resolveOptions({
+        apiRootPath: '/v1',
+      }).apiRootPath
+    ).toEqual(expected)
+
+    expect(
+      resolveOptions({
+        apiRootPath: 'v1/',
+      }).apiRootPath
+    ).toEqual(expected)
+  })
+
+  it('moves `logger` to `fastifyServerOptions.logger`', () => {
+    const resolvedOptions = resolveOptions({
+      logger: { level: 'info' },
+    })
+
+    expect(resolvedOptions).toMatchObject({
+      fastifyServerOptions: {
+        logger: { level: 'info' },
+      },
+    })
+  })
+
+  it('`logger` overwrites `fastifyServerOptions.logger`', () => {
+    const resolvedOptions = resolveOptions({
+      logger: false,
+      fastifyServerOptions: {
+        // @ts-expect-error this is invalid TS but valid JS
+        logger: true,
+      },
+    })
+
+    expect(resolvedOptions).toMatchObject({
+      fastifyServerOptions: {
+        logger: false,
+      },
+    })
+  })
+
+  it('`DEFAULT_CREATE_SERVER_OPTIONS` overwrites `fastifyServerOptions.logger`', () => {
+    const resolvedOptions = resolveOptions({
+      fastifyServerOptions: {
+        // @ts-expect-error this is invalid TS but valid JS
+        logger: true,
+      },
+    })
+
+    expect(resolvedOptions).toMatchObject({
+      fastifyServerOptions: {
+        logger: DEFAULT_CREATE_SERVER_OPTIONS.logger,
+      },
+    })
+  })
+
+  it('parses `--port`', () => {
+    expect(resolveOptions({}, ['--port', '8930']).port).toEqual(8930)
+  })
+
+  it("throws if `--port` can't be converted to an integer", () => {
+    expect(() => {
+      resolveOptions({}, ['--port', 'eight-nine-ten'])
+    }).toThrowErrorMatchingInlineSnapshot(`"\`port\` must be an integer"`)
+  })
+
+  it('parses `--apiRootPath`', () => {
+    expect(resolveOptions({}, ['--apiRootPath', 'foo']).apiRootPath).toEqual(
+      '/foo/'
+    )
+  })
+
+  it('the `--apiRootPath` flag has precedence', () => {
+    expect(
+      resolveOptions({ apiRootPath: 'foo' }, ['--apiRootPath', 'bar'])
+        .apiRootPath
+    ).toEqual('/bar/')
+  })
+})

packages/api-server/src/cliHandlers.ts

@@ -59,7 +59,6 @@ export const apiServerHandler = async (options: ApiServerArgs) => {
   process.stdout.write(c.dim(c.italic('Starting API Server...\n')))
 
   if (loadEnvFiles) {
-    // @ts-expect-error for some reason ts can't find the types here but can find them for other packages
     const { config } = await import('dotenv-defaults')
 
     config({
@@ -197,3 +196,6 @@ function isFullyQualifiedUrl(url: string) {
     return false
   }
 }
+
+// Temporarily here till we refactor server code
+export { createServer } from './createServer'