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

Update url prop handling for pages with new data methods #10653

Merged
merged 16 commits into from Feb 26, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions errors/reserved-page-prop.md
@@ -0,0 +1,9 @@
# Reserved Page Prop

#### Why This Error Occurred

In a page's `getInitialProps` a reserved prop was returned. Currently the only reserved page prop is `url` for legacy reasons.

#### Possible Ways to Fix It

Change the name of the prop returned from `getInitialProps` to any other name.
10 changes: 6 additions & 4 deletions packages/next/build/babel/plugins/next-ssg-transform.ts
@@ -1,10 +1,12 @@
import { NodePath, PluginObj } from '@babel/core'
import * as BabelTypes from '@babel/types'
import { SERVER_PROPS_SSG_CONFLICT } from '../../../lib/constants'
import {
STATIC_PROPS_ID,
SERVER_PROPS_ID,
} from '../../../next-server/lib/constants'

const pageComponentVar = '__NEXT_COMP'
const prerenderId = '__N_SSG'
const serverPropsId = '__N_SSP'

export const EXPORT_NAME_GET_STATIC_PROPS = 'unstable_getStaticProps'
export const EXPORT_NAME_GET_STATIC_PATHS = 'unstable_getStaticPaths'
Expand Down Expand Up @@ -53,7 +55,7 @@ function decorateSsgExport(
'=',
t.memberExpression(
t.identifier(pageComponentVar),
t.identifier(state.isPrerender ? prerenderId : serverPropsId)
t.identifier(state.isPrerender ? STATIC_PROPS_ID : SERVER_PROPS_ID)
),
t.booleanLiteral(true)
),
Expand All @@ -80,7 +82,7 @@ function decorateSsgExport(
'=',
t.memberExpression(
t.identifier((defaultSpecifier as any).local.name),
t.identifier(state.isPrerender ? prerenderId : serverPropsId)
t.identifier(state.isPrerender ? STATIC_PROPS_ID : SERVER_PROPS_ID)
),
t.booleanLiteral(true)
),
Expand Down
2 changes: 2 additions & 0 deletions packages/next/next-server/lib/constants.ts
Expand Up @@ -34,3 +34,5 @@ export const ROUTE_NAME_REGEX = /^static[/\\][^/\\]+[/\\]pages[/\\](.*)\.js$/
export const SERVERLESS_ROUTE_NAME_REGEX = /^pages[/\\](.*)\.js$/
export const TEMPORARY_REDIRECT_STATUS = 307
export const PERMANENT_REDIRECT_STATUS = 308
export const STATIC_PROPS_ID = '__N_SSG'
export const SERVER_PROPS_ID = '__N_SSP'
51 changes: 46 additions & 5 deletions packages/next/next-server/server/load-components.ts
Expand Up @@ -5,6 +5,8 @@ import {
CLIENT_STATIC_FILES_PATH,
REACT_LOADABLE_MANIFEST,
SERVER_DIRECTORY,
STATIC_PROPS_ID,
SERVER_PROPS_ID,
} from '../lib/constants'
import { join } from 'path'
import { requirePage } from './require'
Expand All @@ -16,6 +18,20 @@ export function interopDefault(mod: any) {
return mod.default || mod
}

function addComponentPropsId(
Component: any,
getStaticProps: any,
getServerProps: any
) {
// Mark the component with the SSG or SSP id here since we don't run
// the SSG babel transform for server mode
if (getStaticProps) {
Component[STATIC_PROPS_ID] = true
} else if (getServerProps) {
Component[SERVER_PROPS_ID] = true
}
}

export type ManifestItem = {
id: number | string
name: string
Expand Down Expand Up @@ -66,11 +82,24 @@ export async function loadComponents(
): Promise<LoadComponentsReturnType> {
if (serverless) {
const Component = await requirePage(pathname, distDir, serverless)
const {
unstable_getStaticProps,
unstable_getStaticPaths,
unstable_getServerProps,
} = Component

addComponentPropsId(
Component,
unstable_getStaticProps,
unstable_getServerProps
)

return {
Component,
pageConfig: Component.config || {},
unstable_getStaticProps: Component.unstable_getStaticProps,
unstable_getStaticPaths: Component.unstable_getStaticPaths,
unstable_getStaticProps,
unstable_getStaticPaths,
unstable_getServerProps,
} as LoadComponentsReturnType
}
const documentPath = join(
Expand Down Expand Up @@ -111,6 +140,18 @@ export async function loadComponents(
interopDefault(AppMod),
])

const {
unstable_getServerProps,
unstable_getStaticProps,
unstable_getStaticPaths,
} = ComponentMod

addComponentPropsId(
Component,
unstable_getStaticProps,
unstable_getServerProps
)

return {
App,
Document,
Expand All @@ -119,8 +160,8 @@ export async function loadComponents(
DocumentMiddleware,
reactLoadableManifest,
pageConfig: ComponentMod.config || {},
unstable_getServerProps: ComponentMod.unstable_getServerProps,
unstable_getStaticProps: ComponentMod.unstable_getStaticProps,
unstable_getStaticPaths: ComponentMod.unstable_getStaticPaths,
unstable_getServerProps,
unstable_getStaticProps,
unstable_getStaticPaths,
}
}
13 changes: 13 additions & 0 deletions packages/next/next-server/server/render.tsx
Expand Up @@ -546,6 +546,19 @@ export async function renderToHTML(
props.pageProps = data.props
;(renderOpts as any).pageData = props
}

if (
!isSpr && // we only show this warning for legacy pages
!unstable_getServerProps &&
process.env.NODE_ENV !== 'production' &&
Object.keys(props?.pageProps || {}).includes('url')
) {
console.warn(
ijjk marked this conversation as resolved.
Show resolved Hide resolved
`The prop \`url\` is a reserved prop in Next.js for legacy reasons and will be overridden on page ${pathname}\n` +
`See more info here: https://err.sh/zeit/next.js/reserved-page-prop`
)
}

// We only need to do this if we want to support calling
// _app's getInitialProps for getServerProps if not this can be removed
if (isDataReq) return props
Expand Down
15 changes: 13 additions & 2 deletions packages/next/pages/_app.tsx
Expand Up @@ -42,8 +42,19 @@ export default class App<P = {}, CP = {}, S = {}> extends React.Component<

render() {
const { router, Component, pageProps } = this.props as AppProps<CP>
const url = createUrl(router)
return <Component {...pageProps} url={url} />

return (
<Component
{...pageProps}
{
// we don't add the legacy URL prop if it's using non-legacy
// methods like getStaticProps and getServerProps
...(!((Component as any).__N_SSG || (Component as any).__N_SSP)
? { url: createUrl(router) }
: {})
}
/>
)
}
}

Expand Down
40 changes: 38 additions & 2 deletions test/integration/getserverprops/test/index.test.js
Expand Up @@ -23,6 +23,7 @@ const nextConfig = join(appDir, 'next.config.js')
let app
let appPort
let buildId
let stderr

const expectedManifestRoutes = () => ({
'/something': {
Expand Down Expand Up @@ -322,6 +323,31 @@ const runTests = (dev = false) => {
})

if (dev) {
it('should not show warning from url prop being returned', async () => {
const urlPropPage = join(appDir, 'pages/url-prop.js')
await fs.writeFile(
urlPropPage,
`
export async function unstable_getServerProps() {
return {
props: {
url: 'something'
}
}
}

export default ({ url }) => <p>url: {url}</p>
`
)

const html = await renderViaHTTP(appPort, '/url-prop')
await fs.remove(urlPropPage)
expect(stderr).not.toMatch(
/The prop `url` is a reserved prop in Next.js for legacy reasons and will be overridden on page \/url-prop/
)
expect(html).toMatch(/url:.*?something/)
})

it('should show error for extra keys returned from getServerProps', async () => {
const html = await renderViaHTTP(appPort, '/invalid-keys')
expect(html).toContain(
Expand Down Expand Up @@ -365,8 +391,13 @@ const runTests = (dev = false) => {
describe('unstable_getServerProps', () => {
describe('dev mode', () => {
beforeAll(async () => {
stderr = ''
appPort = await findPort()
app = await launchApp(appDir, appPort)
app = await launchApp(appDir, appPort, {
onStderr(msg) {
stderr += msg
},
})
buildId = 'development'
})
afterAll(() => killApp(app))
Expand All @@ -382,8 +413,13 @@ describe('unstable_getServerProps', () => {
'utf8'
)
await nextBuild(appDir)
stderr = ''
appPort = await findPort()
app = await nextStart(appDir, appPort)
app = await nextStart(appDir, appPort, {
onStderr(msg) {
stderr += msg
},
})
buildId = await fs.readFile(join(appDir, '.next/BUILD_ID'), 'utf8')
})
afterAll(() => killApp(app))
Expand Down
25 changes: 25 additions & 0 deletions test/integration/prerender/test/index.test.js
Expand Up @@ -428,6 +428,31 @@ const runTests = (dev = false, looseMode = false) => {
// )
// })

it('should not show warning from url prop being returned', async () => {
const urlPropPage = join(appDir, 'pages/url-prop.js')
await fs.writeFile(
urlPropPage,
`
export async function unstable_getStaticProps() {
return {
props: {
url: 'something'
}
}
}

export default ({ url }) => <p>url: {url}</p>
`
)

const html = await renderViaHTTP(appPort, '/url-prop')
await fs.remove(urlPropPage)
expect(stderr).not.toMatch(
/The prop `url` is a reserved prop in Next.js for legacy reasons and will be overridden on page \/url-prop/
)
expect(html).toMatch(/url:.*?something/)
})

it('should always show fallback for page not in getStaticPaths', async () => {
const html = await renderViaHTTP(appPort, '/blog/post-321')
const $ = cheerio.load(html)
Expand Down