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

Polish logging segments of page route #57834

Merged
merged 6 commits into from
Nov 1, 2023
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
11 changes: 11 additions & 0 deletions packages/next/src/build/output/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,18 @@ export function watchCompilers(
})
}

const internalSegments = ['[[...__metadata_id__]]', '[__metadata_id__]']
export function reportTrigger(trigger: string) {
for (const segment of internalSegments) {
if (trigger.includes(segment)) {
trigger = trigger.replace(segment, '')
}
}

if (trigger.length > 1 && trigger.endsWith('/')) {
trigger = trigger.slice(0, -1)
}

buildStore.setState({
trigger,
})
Expand Down
4 changes: 3 additions & 1 deletion packages/next/src/server/dev/on-demand-entry-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import { HMR_ACTIONS_SENT_TO_BROWSER } from './hot-reloader-types'
import { isAppPageRouteDefinition } from '../future/route-definitions/app-page-route-definition'
import { scheduleOnNextTick } from '../../lib/scheduler'
import { Batcher } from '../../lib/batcher'
import { normalizeAppPath } from '../../shared/lib/router/utils/app-paths'

const debug = createDebug('next:on-demand-entry-handler')

Expand Down Expand Up @@ -832,7 +833,8 @@ export function onDemandEntryHandler({
const hasNewEntry = addedValues.some((entry) => entry.newEntry)

if (hasNewEntry) {
reportTrigger(route.page)
const routePage = isApp ? route.page : normalizeAppPath(route.page)
reportTrigger(routePage)
}

if (entriesThatShouldBeInvalidated.length > 0) {
Expand Down
2 changes: 1 addition & 1 deletion test/development/basic/barrel-optimization.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ describe('optimizePackageImports', () => {

const modules = [
...logs.matchAll(
/Compiled (\/[\w-]+)*\s*in \d+(\.\d+)?(s|ms) \((\d+) modules\)/g
/Compiled (\/[\w-]*)*\s*in \d+(\.\d+)?(s|ms) \((\d+) modules\)/g
),
]

Expand Down
63 changes: 63 additions & 0 deletions test/e2e/app-dir/logging/app/default-cache/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
export const fetchCache = 'default-cache'

export default async function Page() {
const dataNoCache = await fetch(
'https://next-data-api-endpoint.vercel.app/api/random?no-cache',
{
cache: 'no-cache',
}
).then((res) => res.text())

const dataForceCache = await fetch(
'https://next-data-api-endpoint.vercel.app/api/random?force-cache',
{
cache: 'force-cache',
}
).then((res) => res.text())

const dataRevalidate0 = await fetch(
'https://next-data-api-endpoint.vercel.app/api/random?revalidate-0',
{
next: {
revalidate: 0,
},
}
).then((res) => res.text())

const dataRevalidateCache = await fetch(
'https://next-data-api-endpoint.vercel.app/api/random?revalidate-3',
{
next: {
revalidate: 3,
},
}
).then((res) => res.text())

const dataRevalidateAndFetchCache = await fetch(
'https://next-data-api-endpoint.vercel.app/api/random?revalidate-3-force-cache',
{
next: {
revalidate: 3,
},
cache: 'force-cache',
}
).then((res) => res.text())

const dataAutoCache = await fetch(
'https://next-data-api-endpoint.vercel.app/api/random?auto-cache'
).then((res) => res.text())

return (
<>
<p>/force-cache</p>
<p id="data-no-cache">"cache: no-cache" {dataNoCache}</p>
<p id="data-force-cache">"cache: force-cache" {dataForceCache}</p>
<p id="data-revalidate-0">"revalidate: 0" {dataRevalidate0}</p>
<p id="data-revalidate-cache">"revalidate: 3" {dataRevalidateCache}</p>
<p id="data-revalidate-and-fetch-cache">
"revalidate: 3 and cache: force-cache" {dataRevalidateAndFetchCache}
</p>
<p id="data-auto-cache">"auto cache" {dataAutoCache}</p>
</>
)
}
22 changes: 22 additions & 0 deletions test/e2e/app-dir/logging/app/dynamic/[slug]/icon.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { ImageResponse } from 'next/og'

export default function icon({ params, id }) {
return new ImageResponse(
(
<div
style={{
width: '100%',
height: '100%',
display: 'flex',
alignItems: 'center',
justifyContent: 'center',
fontSize: 88,
background: '#000',
color: '#fafafa',
}}
>
Apple {params.size} {id}
</div>
)
)
}
7 changes: 7 additions & 0 deletions test/e2e/app-dir/logging/app/layout.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default function Layout({ children }) {
return (
<html>
<body>{children}</body>
</html>
)
}
3 changes: 3 additions & 0 deletions test/e2e/app-dir/logging/app/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return 'hello world'
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import path from 'path'
import stripAnsi from 'strip-ansi'
import { check } from 'next-test-utils'
import { createNextDescribe, FileRef } from 'e2e-utils'
import { createNextDescribe } from 'e2e-utils'

function parseLogsFromCli(cliOutput: string) {
const logs = stripAnsi(cliOutput)
Expand Down Expand Up @@ -37,22 +36,14 @@ function parseLogsFromCli(cliOutput: string) {
}

createNextDescribe(
'app-dir - data fetching with cache logging',
'app-dir - logging',
{
skipDeployment: true,
files: {
'app/layout.js': new FileRef(path.join(__dirname, 'app/layout.js')),
'app/default-cache/page.js': new FileRef(
path.join(__dirname, 'app/default-cache/page.js')
),
'next.config.js': `module.exports = {
logging: { fetches: { fullUrl: true } }
}`,
},
files: __dirname,
},
({ next, isNextDev }) => {
function runTests({ hasLogging }: { hasLogging: boolean }) {
if (hasLogging) {
function runTests({ withFetchesLogging }: { withFetchesLogging: boolean }) {
if (withFetchesLogging) {
it('should only log requests in dev mode', async () => {
const outputIndex = next.cliOutput.length
await next.fetch('/default-cache')
Expand Down Expand Up @@ -143,10 +134,40 @@ createNextDescribe(
}, 'success')
})
}

if (isNextDev) {
it('should not contain trailing word page for app router routes', async () => {
const logLength = next.cliOutput.length
await next.fetch('/')

await check(() => {
const output = stripAnsi(next.cliOutput.slice(logLength))
expect(output).toContain('/')
expect(output).not.toContain('/page')

return 'success'
}, /success/)
})

it('should not contain metadata internal segments for dynamic metadata routes', async () => {
const logLength = next.cliOutput.length
await next.fetch('/dynamic/big/icon')

await check(() => {
const output = stripAnsi(next.cliOutput.slice(logLength))
expect(output).toContain('/dynamic/[slug]/icon')
expect(output).not.toContain('/(group)')
expect(output).not.toContain('[[...__metadata_id__]]')
expect(output).not.toContain('/route')

return 'success'
}, /success/)
})
}
}

describe('with verbose logging', () => {
runTests({ hasLogging: true })
runTests({ withFetchesLogging: true })
})

describe('with verbose logging for edge runtime', () => {
Expand All @@ -160,7 +181,7 @@ createNextDescribe(
await next.start()
})

runTests({ hasLogging: false })
runTests({ withFetchesLogging: false })
})

describe('with default logging', () => {
Expand All @@ -170,7 +191,7 @@ createNextDescribe(
await next.start()
})

runTests({ hasLogging: false })
runTests({ withFetchesLogging: false })
})
}
)
7 changes: 7 additions & 0 deletions test/e2e/app-dir/logging/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
module.exports = {
logging: {
fetches: {
fullUrl: true,
},
},
}
1 change: 1 addition & 0 deletions test/e2e/app-dir/metadata-dynamic-routes/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ createNextDescribe(
type: $(el).attr('type'),
}
})

// slug is id param from generateImageMetadata
expect(iconUrls).toMatchObject([
{
Expand Down
Loading