Skip to content

Commit

Permalink
chore: show large data warning once per page on prod (#46323)
Browse files Browse the repository at this point in the history
<!--
Thanks for opening a PR! Your contribution is much appreciated.
To make sure your PR is handled as smoothly as possible we request that
you follow the checklist sections below.
Choose the right checklist for the change(s) that you're making:
-->

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [x] Errors have a helpful link attached, see
[`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)

## Related

* Closes #39146 - PR abandoned.

## Details

I was having the same issue at work, as a workaround, we've changed the
`largePageDataBytes` setting as advised but we'd like to keep the
warning ideally. It's something we need to address, but just don't want
to spam our log aggregators while the large data issue isn't resolved.

I believe I've more or less copied what was in the original PR, some
small differences:
* I use `Set` instead of `Map`, please let me know if there is an
advantage to using Map!
* Just to keep code a bit tidier (subjective) put an early return
instead of nesting all the code in an if.
* Added a test to verify only one log appears even when the page is
accessed twice [as requested
here](#39146 (review)).
  • Loading branch information
SethFalco authored Feb 25, 2023
1 parent d9776cc commit adefdd2
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 0 deletions.
12 changes: 12 additions & 0 deletions packages/next/src/pages/_document.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ type HeadHTMLProps = React.DetailedHTMLProps<

type HeadProps = OriginProps & HeadHTMLProps

/** Set of pages that have triggered a large data warning on production mode. */
const largePageDataWarnings = new Set<string>()

function getDocumentFiles(
buildManifest: BuildManifest,
pathname: string,
Expand Down Expand Up @@ -998,13 +1001,22 @@ export class NextScript extends React.Component<OriginProps> {
const { __NEXT_DATA__, largePageDataBytes } = context
try {
const data = JSON.stringify(__NEXT_DATA__)

if (largePageDataWarnings.has(__NEXT_DATA__.page)) {
return htmlEscapeJsonString(data)
}

const bytes =
process.env.NEXT_RUNTIME === 'edge'
? new TextEncoder().encode(data).buffer.byteLength
: Buffer.from(data).byteLength
const prettyBytes = require('../lib/pretty-bytes').default

if (largePageDataBytes && bytes > largePageDataBytes) {
if (process.env.NODE_ENV === 'production') {
largePageDataWarnings.add(__NEXT_DATA__.page)
}

console.warn(
`Warning: data for page "${__NEXT_DATA__.page}"${
__NEXT_DATA__.page === context.dangerousAsPath
Expand Down
39 changes: 39 additions & 0 deletions test/e2e/prerender.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -967,6 +967,39 @@ describe('Prerender', () => {
)
})

if ((global as any).isNextDev) {
it('should show warning every time page with large amount of page data is returned', async () => {
await renderViaHTTP(next.url, '/large-page-data-ssr')
await check(
() => next.cliOutput,
/Warning: data for page "\/large-page-data-ssr" is 256 kB which exceeds the threshold of 128 kB, this amount of data can reduce performance/
)

const outputIndex = next.cliOutput.length
await renderViaHTTP(next.url, '/large-page-data-ssr')
await check(
() => next.cliOutput.slice(outputIndex),
/Warning: data for page "\/large-page-data-ssr" is 256 kB which exceeds the threshold of 128 kB, this amount of data can reduce performance/
)
})
}

if ((global as any).isNextStart) {
it('should only show warning once per page when large amount of page data is returned', async () => {
await renderViaHTTP(next.url, '/large-page-data-ssr')
await check(
() => next.cliOutput,
/Warning: data for page "\/large-page-data-ssr" is 256 kB which exceeds the threshold of 128 kB, this amount of data can reduce performance/
)

const outputIndex = next.cliOutput.length
await renderViaHTTP(next.url, '/large-page-data-ssr')
expect(next.cliOutput.slice(outputIndex)).not.toInclude(
'Warning: data for page'
)
})
}

if ((global as any).isNextDev) {
it('should not show warning from url prop being returned', async () => {
const urlPropPage = 'pages/url-prop.js'
Expand Down Expand Up @@ -1483,6 +1516,12 @@ describe('Prerender', () => {
)}\\/large-page-data.json$`,
page: '/large-page-data',
},
{
dataRouteRegex: `^\\/_next\\/data\\/${escapeRegex(
next.buildId
)}\\/large-page-data-ssr.json$`,
page: '/large-page-data-ssr',
},
{
namedDataRouteRegex: `^/_next/data/${escapeRegex(
next.buildId
Expand Down
25 changes: 25 additions & 0 deletions test/e2e/prerender/pages/large-page-data-ssr.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import React from 'react'
import Link from 'next/link'

export function getServerSideProps() {
return {
props: {
lotsOfData: new Array(256 * 1000).fill('a').join(''),
},
}
}

export default (props) => {
return (
<>
<p>lots of data returned</p>
<Link href="/" id="home">
to home
</Link>
<br />
<Link href="/another" id="another">
to another
</Link>
</>
)
}

0 comments on commit adefdd2

Please sign in to comment.