Skip to content

Commit cdc7f01

Browse files
Joey Teppermanijjktimneutkens
authored
Add warning when a page is rendered without a starting / (vercel#11418)
* Add error/warning when a page is rendered without a / Throws an error for development and gives a warning in production * Add tests for error when rendering without starting slash * Update to always warn and add err.sh * Update errors/render-no-starting-slash.md Co-authored-by: JJ Kasper <[email protected]> Co-authored-by: Tim Neutkens <[email protected]>
1 parent 8f4e265 commit cdc7f01

File tree

5 files changed

+77
-4
lines changed

5 files changed

+77
-4
lines changed

errors/render-no-starting-slash.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
# Rendering Without Starting Slash
2+
3+
#### Why This Error Occurred
4+
5+
When calling `app.render(req, res, path)` the `path` did not begin with a slash (`/`). This causes unexpected behavior and should be corrected.
6+
7+
#### Possible Ways to Fix It
8+
9+
Make sure the `path` being passed to `app.render` always starts with a slash (`/`)

packages/next/next-server/server/next-server.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -786,6 +786,12 @@ export default class Server {
786786
query: ParsedUrlQuery = {},
787787
parsedUrl?: UrlWithParsedQuery
788788
): Promise<void> {
789+
if (!pathname.startsWith('/')) {
790+
console.warn(
791+
`Cannot render page with path "${pathname}", did you mean "/${pathname}"?. See more info here: https://err.sh/next.js/render-no-starting-slash`
792+
)
793+
}
794+
789795
const url: any = req.url
790796

791797
if (

test/integration/custom-server/server.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ const app = next({ dev, dir })
99
const handleNextRequests = app.getRequestHandler()
1010

1111
app.prepare().then(() => {
12-
const server = new http.Server((req, res) => {
12+
const server = new http.Server(async (req, res) => {
1313
if (req.url === '/no-query') {
1414
return app.render(req, res, '/no-query')
1515
}
@@ -36,6 +36,14 @@ app.prepare().then(() => {
3636
return app.render(req, res, '/static/hello.text')
3737
}
3838

39+
if (/no-slash/.test(req.url)) {
40+
try {
41+
await app.render(req, res, 'dashboard')
42+
} catch (err) {
43+
res.end(err.message)
44+
}
45+
}
46+
3947
handleNextRequests(req, res)
4048
})
4149

test/integration/custom-server/test/index.test.js

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
fetchViaHTTP,
1313
check,
1414
File,
15+
nextBuild,
1516
} from 'next-test-utils'
1617

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

2425
const context = {}
2526

26-
const startServer = async (optEnv = {}) => {
27+
const startServer = async (optEnv = {}, opts) => {
2728
const scriptPath = join(appDir, 'server.js')
2829
context.appPort = appPort = await getPort()
2930
const env = Object.assign(
@@ -37,7 +38,8 @@ const startServer = async (optEnv = {}) => {
3738
scriptPath,
3839
/ready on/i,
3940
env,
40-
/ReferenceError: options is not defined/
41+
/ReferenceError: options is not defined/,
42+
opts
4143
)
4244
}
4345

@@ -142,4 +144,43 @@ describe('Custom Server', () => {
142144
}
143145
})
144146
})
147+
148+
describe('Error when rendering without starting slash', () => {
149+
afterEach(() => killApp(server))
150+
151+
it('should warn in dev mode', async () => {
152+
let stderr = ''
153+
await startServer(
154+
{},
155+
{
156+
onStderr(msg) {
157+
stderr += msg || ''
158+
},
159+
}
160+
)
161+
const html = await renderViaHTTP(appPort, '/no-slash')
162+
expect(html).toContain('made it to dashboard')
163+
expect(stderr).toContain('Cannot render page with path "dashboard"')
164+
})
165+
166+
it('should warn in production mode', async () => {
167+
const { code } = await nextBuild(appDir)
168+
expect(code).toBe(0)
169+
170+
let stderr = ''
171+
172+
await startServer(
173+
{ NODE_ENV: 'production' },
174+
{
175+
onStderr(msg) {
176+
stderr += msg || ''
177+
},
178+
}
179+
)
180+
181+
const html = await renderViaHTTP(appPort, '/no-slash')
182+
expect(html).toContain('made it to dashboard')
183+
expect(stderr).toContain('Cannot render page with path "dashboard"')
184+
})
185+
})
145186
})

test/lib/next-test-utils.js

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ export function initNextServerScript(
2121
scriptPath,
2222
successRegexp,
2323
env,
24-
failRegexp
24+
failRegexp,
25+
opts
2526
) {
2627
return new Promise((resolve, reject) => {
2728
const instance = spawn('node', [scriptPath], { env })
@@ -32,6 +33,10 @@ export function initNextServerScript(
3233
resolve(instance)
3334
}
3435
process.stdout.write(message)
36+
37+
if (opts && opts.onStdout) {
38+
opts.onStdout(message.toString())
39+
}
3540
}
3641

3742
function handleStderr(data) {
@@ -41,6 +46,10 @@ export function initNextServerScript(
4146
return reject(new Error('received failRegexp'))
4247
}
4348
process.stderr.write(message)
49+
50+
if (opts && opts.onStderr) {
51+
opts.onStderr(message.toString())
52+
}
4453
}
4554

4655
instance.stdout.on('data', handleStdout)

0 commit comments

Comments
 (0)