Skip to content

Commit

Permalink
Add warning when a page is rendered without a starting / (vercel#11418)
Browse files Browse the repository at this point in the history
* 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]>
  • Loading branch information
3 people authored Apr 6, 2020
1 parent 8f4e265 commit cdc7f01
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 4 deletions.
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

0 comments on commit cdc7f01

Please sign in to comment.