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

Fix bug where pages refresh periodically for custom server #11418

Merged
merged 6 commits into from
Apr 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions errors/render-no-starting-slash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Rendering Without Starting Slash

#### Why This Error Occurred

When calling `app.render(req, res, path)` the `path` did not begin with a slash (`/`). This causes unexpected behavior and should be corrected.

#### Possible Ways to Fix It

Make sure the `path` being passed to `app.render` always starts with a slash (`/`)
6 changes: 6 additions & 0 deletions packages/next/next-server/server/next-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -786,6 +786,12 @@ export default class Server {
query: ParsedUrlQuery = {},
parsedUrl?: UrlWithParsedQuery
): Promise<void> {
if (!pathname.startsWith('/')) {
console.warn(
`Cannot render page with path "${pathname}", did you mean "/${pathname}"?. See more info here: https://err.sh/next.js/render-no-starting-slash`
)
}

const url: any = req.url

if (
Expand Down
10 changes: 9 additions & 1 deletion test/integration/custom-server/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const app = next({ dev, dir })
const handleNextRequests = app.getRequestHandler()

app.prepare().then(() => {
const server = new http.Server((req, res) => {
const server = new http.Server(async (req, res) => {
if (req.url === '/no-query') {
return app.render(req, res, '/no-query')
}
Expand All @@ -36,6 +36,14 @@ app.prepare().then(() => {
return app.render(req, res, '/static/hello.text')
}

if (/no-slash/.test(req.url)) {
try {
await app.render(req, res, 'dashboard')
} catch (err) {
res.end(err.message)
}
}

handleNextRequests(req, res)
})

Expand Down
45 changes: 43 additions & 2 deletions test/integration/custom-server/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
fetchViaHTTP,
check,
File,
nextBuild,
} from 'next-test-utils'

const appDir = join(__dirname, '../')
Expand All @@ -23,7 +24,7 @@ jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 2

const context = {}

const startServer = async (optEnv = {}) => {
const startServer = async (optEnv = {}, opts) => {
const scriptPath = join(appDir, 'server.js')
context.appPort = appPort = await getPort()
const env = Object.assign(
Expand All @@ -37,7 +38,8 @@ const startServer = async (optEnv = {}) => {
scriptPath,
/ready on/i,
env,
/ReferenceError: options is not defined/
/ReferenceError: options is not defined/,
opts
)
}

Expand Down Expand Up @@ -142,4 +144,43 @@ describe('Custom Server', () => {
}
})
})

describe('Error when rendering without starting slash', () => {
afterEach(() => killApp(server))

it('should warn in dev mode', async () => {
let stderr = ''
await startServer(
{},
{
onStderr(msg) {
stderr += msg || ''
},
}
)
const html = await renderViaHTTP(appPort, '/no-slash')
expect(html).toContain('made it to dashboard')
expect(stderr).toContain('Cannot render page with path "dashboard"')
})

it('should warn in production mode', async () => {
const { code } = await nextBuild(appDir)
expect(code).toBe(0)

let stderr = ''

await startServer(
{ NODE_ENV: 'production' },
{
onStderr(msg) {
stderr += msg || ''
},
}
)

const html = await renderViaHTTP(appPort, '/no-slash')
expect(html).toContain('made it to dashboard')
expect(stderr).toContain('Cannot render page with path "dashboard"')
})
})
})
11 changes: 10 additions & 1 deletion test/lib/next-test-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ export function initNextServerScript(
scriptPath,
successRegexp,
env,
failRegexp
failRegexp,
opts
) {
return new Promise((resolve, reject) => {
const instance = spawn('node', [scriptPath], { env })
Expand All @@ -32,6 +33,10 @@ export function initNextServerScript(
resolve(instance)
}
process.stdout.write(message)

if (opts && opts.onStdout) {
opts.onStdout(message.toString())
}
}

function handleStderr(data) {
Expand All @@ -41,6 +46,10 @@ export function initNextServerScript(
return reject(new Error('received failRegexp'))
}
process.stderr.write(message)

if (opts && opts.onStderr) {
opts.onStderr(message.toString())
}
}

instance.stdout.on('data', handleStdout)
Expand Down