Skip to content

Commit

Permalink
feat(server file): add createServer (redwoodjs#9845)
Browse files Browse the repository at this point in the history
This PR brings the server file out of experimental by implementing a
`createServer` function. This is a continuation of the work started in
redwoodjs#8119.

This API was designed in response to the feedback to redwoodjs#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>
  • Loading branch information
3 people committed Jan 21, 2024
1 parent 9245fe7 commit 41ac728
Show file tree
Hide file tree
Showing 36 changed files with 1,266 additions and 358 deletions.
1 change: 1 addition & 0 deletions packages/api-server/ambient.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
declare module 'dotenv-defaults'
1 change: 1 addition & 0 deletions packages/api-server/dist.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ describe('dist', () => {
"type": "string",
},
},
"createServer": [Function],
"webCliOptions": {
"apiHost": {
"alias": "api-host",
Expand Down
9 changes: 9 additions & 0 deletions packages/api-server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
330 changes: 330 additions & 0 deletions packages/api-server/src/__tests__/createServer.test.ts
Original file line number Diff line number Diff line change
@@ -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/')
})
})
4 changes: 3 additions & 1 deletion packages/api-server/src/cliHandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -197,3 +196,6 @@ function isFullyQualifiedUrl(url: string) {
return false
}
}

// Temporarily here till we refactor server code
export { createServer } from './createServer'
Loading

0 comments on commit 41ac728

Please sign in to comment.