work on options

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

@@ -7,9 +7,8 @@ import { getConfig } from '@redwoodjs/project-config'
 
 import {
   createServer,
-  resolveCreateServerOptions,
+  resolveOptions,
   DEFAULT_CREATE_SERVER_OPTIONS,
-  parseArgs,
 } from '../createServer'
 
 // Set up RWJS_CWD.
@@ -57,8 +56,6 @@ describe('createServer', () => {
     })
 
     it('warns about server.config.js', async () => {
-      console.warn = jest.fn()
-
       await createServer()
 
       expect(consoleWarnSpy.mock.calls[0][0]).toMatchInlineSnapshot(`
@@ -228,9 +225,9 @@ describe('createServer', () => {
   })
 })
 
-describe('resolveCreateServerOptions', () => {
+describe('resolveOptions', () => {
   it('nothing passed', () => {
-    const resolvedOptions = resolveCreateServerOptions()
+    const resolvedOptions = resolveOptions()
 
     expect(resolvedOptions).toEqual({
       apiRootPath: DEFAULT_CREATE_SERVER_OPTIONS.apiRootPath,
@@ -239,33 +236,34 @@ describe('resolveCreateServerOptions', () => {
           DEFAULT_CREATE_SERVER_OPTIONS.fastifyServerOptions.requestTimeout,
         logger: DEFAULT_CREATE_SERVER_OPTIONS.logger,
       },
+      port: 8911,
     })
   })
 
   it('ensures `apiRootPath` has slashes', () => {
     const expected = '/v1/'
 
     expect(
-      resolveCreateServerOptions({
+      resolveOptions({
         apiRootPath: 'v1',
       }).apiRootPath
     ).toEqual(expected)
 
     expect(
-      resolveCreateServerOptions({
+      resolveOptions({
         apiRootPath: '/v1',
       }).apiRootPath
     ).toEqual(expected)
 
     expect(
-      resolveCreateServerOptions({
+      resolveOptions({
         apiRootPath: 'v1/',
       }).apiRootPath
     ).toEqual(expected)
   })
 
   it('moves `logger` to `fastifyServerOptions.logger`', () => {
-    const resolvedOptions = resolveCreateServerOptions({
+    const resolvedOptions = resolveOptions({
       logger: { level: 'info' },
     })
 
@@ -277,7 +275,7 @@ describe('resolveCreateServerOptions', () => {
   })
 
   it('`logger` overwrites `fastifyServerOptions.logger`', () => {
-    const resolvedOptions = resolveCreateServerOptions({
+    const resolvedOptions = resolveOptions({
       logger: false,
       fastifyServerOptions: {
         // @ts-expect-error this is invalid TS but valid JS
@@ -293,7 +291,7 @@ describe('resolveCreateServerOptions', () => {
   })
 
   it('`DEFAULT_CREATE_SERVER_OPTIONS` overwrites `fastifyServerOptions.logger`', () => {
-    const resolvedOptions = resolveCreateServerOptions({
+    const resolvedOptions = resolveOptions({
       fastifyServerOptions: {
         // @ts-expect-error this is invalid TS but valid JS
         logger: true,
@@ -306,21 +304,27 @@ describe('resolveCreateServerOptions', () => {
       },
     })
   })
-})
 
-describe('parseArgs', () => {
   it('parses `--port`', () => {
-    expect(parseArgs(['--port', '8930']).port).toEqual(8930)
+    expect(resolveOptions({}, ['--port', '8930']).port).toEqual(8930)
   })
 
-  it("throws if `--port` can't be converted to a number", () => {
+  it("throws if `--port` can't be converted to an integer", () => {
     expect(() => {
-      parseArgs(['--port', 'eight-nine-ten'])
-    }).toThrowErrorMatchingInlineSnapshot(`"\`--port\` must be an integer"`)
+      resolveOptions({}, ['--port', 'eight-nine-ten'])
+    }).toThrowErrorMatchingInlineSnapshot(`"\`port\` must be an integer"`)
   })
 
-  it('returns an empty object if passed no args', () => {
-    const args = parseArgs([])
-    expect(args).toEqual({})
+  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/createServer.ts

@@ -1,6 +1,6 @@
 import fs from 'fs'
 import path from 'path'
-import { parseArgs as _parseArgs } from 'util'
+import { parseArgs } from 'util'
 
 import fastifyUrlData from '@fastify/url-data'
 import c from 'ansi-colors'
@@ -89,8 +89,7 @@ export interface CreateServerOptions {
  * ```
  */
 export async function createServer(options: CreateServerOptions = {}) {
-  const { apiRootPath, fastifyServerOptions } =
-    resolveCreateServerOptions(options)
+  const { apiRootPath, fastifyServerOptions, port } = resolveOptions(options)
 
   // Warn about `api/server.config.js`
   const serverConfigPath = path.join(
@@ -188,25 +187,39 @@ export async function createServer(options: CreateServerOptions = {}) {
    * - [api].port in redwood.toml
    */
   server.start = (options: StartOptions = {}) => {
-    return server.listen(resolveStartOptions(options))
+    return server.listen({
+      ...options,
+      port,
+      host: process.env.NODE_ENV === 'production' ? '0.0.0.0' : '::',
+    })
   }
 
   return server
 }
 
-type ResolvedCreateServerOptions = Required<
+type ResolvedOptions = Required<
   Omit<CreateServerOptions, 'logger' | 'fastifyServerOptions'> & {
     fastifyServerOptions: FastifyServerOptions
+    port: number
   }
 >
 
-export function resolveCreateServerOptions(
-  options: CreateServerOptions = {}
-): ResolvedCreateServerOptions {
+export function resolveOptions(
+  options: CreateServerOptions = {},
+  args?: string[]
+) {
   options.logger ??= DEFAULT_CREATE_SERVER_OPTIONS.logger
 
+  let defaultPort: number | undefined
+
+  if (process.env.REDWOOD_API_PORT === undefined) {
+    defaultPort = getConfig().api.port
+  } else {
+    defaultPort = parseInt(process.env.REDWOOD_API_PORT)
+  }
+
   // Set defaults.
-  const resolvedOptions: ResolvedCreateServerOptions = {
+  const resolvedOptions: ResolvedOptions = {
     apiRootPath:
       options.apiRootPath ?? DEFAULT_CREATE_SERVER_OPTIONS.apiRootPath,
 
@@ -215,14 +228,51 @@ export function resolveCreateServerOptions(
         DEFAULT_CREATE_SERVER_OPTIONS.fastifyServerOptions.requestTimeout,
       logger: options.logger ?? DEFAULT_CREATE_SERVER_OPTIONS.logger,
     },
+
+    port: defaultPort,
   }
 
   // Merge fastifyServerOptions.
   resolvedOptions.fastifyServerOptions.requestTimeout ??=
     DEFAULT_CREATE_SERVER_OPTIONS.fastifyServerOptions.requestTimeout
   resolvedOptions.fastifyServerOptions.logger = options.logger
 
-  // Ensure the apiRootPath begins and ends with a slash.
+  const { values } = parseArgs({
+    options: {
+      apiRootPath: {
+        type: 'string',
+      },
+      port: {
+        type: 'string',
+        short: 'p',
+      },
+    },
+
+    // When running Jest, `process.argv` is...
+    //
+    // ```js
+    // [
+    //    'path/to/node'
+    //    'path/to/jest.js'
+    //    'file/under/test.js'
+    // ]
+    // ```
+    //
+    // `parseArgs` strips the first two, leaving the third, which is interpreted as a positional argument.
+    // Which fails our options. We'd still like to be strict, but can't do it for tests.
+    strict: process.env.NODE_ENV === 'test' ? false : true,
+    ...(args && { args }),
+  })
+
+  if (values.apiRootPath && typeof values.apiRootPath !== 'string') {
+    throw new Error('`apiRootPath` must be a string')
+  }
+
+  if (values.apiRootPath) {
+    resolvedOptions.apiRootPath = values.apiRootPath
+  }
+
+  // Format `apiRootPath`
   if (resolvedOptions.apiRootPath.charAt(0) !== '/') {
     resolvedOptions.apiRootPath = `/${resolvedOptions.apiRootPath}`
   }
@@ -235,6 +285,14 @@ export function resolveCreateServerOptions(
     resolvedOptions.apiRootPath = `${resolvedOptions.apiRootPath}/`
   }
 
+  if (values.port) {
+    resolvedOptions.port = +values.port
+
+    if (isNaN(resolvedOptions.port)) {
+      throw new Error('`port` must be an integer')
+    }
+  }
+
   return resolvedOptions
 }
 
@@ -260,7 +318,6 @@ export interface RedwoodFastifyAPIOptions {
   }
 }
 
-// TODO: isolate context.
 export async function redwoodFastifyFunctions(
   fastify: FastifyInstance,
   opts: RedwoodFastifyAPIOptions,
@@ -286,75 +343,3 @@ export async function redwoodFastifyFunctions(
 
   done()
 }
-
-function resolveStartOptions(
-  options: Omit<FastifyListenOptions, 'port' | 'host'>
-): FastifyListenOptions {
-  const resolvedOptions: FastifyListenOptions = options
-
-  // Right now, `host` isn't configurable and is set based on `NODE_ENV` for Docker.
-  resolvedOptions.host =
-    process.env.NODE_ENV === 'production' ? '0.0.0.0' : '::'
-
-  const args = parseArgs()
-
-  if (args.port) {
-    resolvedOptions.port = args.port
-  } else {
-    if (process.env.REDWOOD_API_PORT === undefined) {
-      resolvedOptions.port = getConfig().api.port
-    } else {
-      resolvedOptions.port = parseInt(process.env.REDWOOD_API_PORT)
-    }
-  }
-
-  return resolvedOptions
-}
-
-/**
- * The `args` parameter is just for testing. `_parseArgs` defaults to `process.argv`, which is what we want.
- * This is also exported just for testing.
- */
-export function parseArgs(args?: string[]) {
-  const options = {
-    apiRootPath: {
-      type: 'string',
-    },
-    port: {
-      type: 'string',
-      short: 'p',
-    },
-  }
-
-  const { values } = _parseArgs({
-    // When running Jest, `process.argv` is...
-    //
-    // ```js
-    // [
-    //    'path/to/node'
-    //    'path/to/jest.js'
-    //    'file/under/test.js'
-    // ]
-    // ```
-    //
-    // `parseArgs` strips the first two, leaving the third, which is interpreted as a positional argument.
-    // Which fails our options. We'd still like to be strict, but can't do it for tests.
-    strict: process.env.NODE_ENV === 'test' ? false : true,
-
-    ...(args && { args }),
-    // @ts-expect-error TODO
-    options,
-  })
-
-  const parsedArgs: { port?: number } = {}
-
-  if (values.port) {
-    parsedArgs.port = +values.port
-
-    if (isNaN(parsedArgs.port)) {
-      throw new Error('`--port` must be an integer')
-    }
-  }
-
-  return parsedArgs
-}