Skip to content

Commit

Permalink
Fix client chunk loading encoding for dynamic route (#57960)
Browse files Browse the repository at this point in the history
We had added encoding the client component assets loaded from RSC manifest that we need to encode them to make sure when they're loaded on server and sent to client, the client will receive the encoded one. But the override of the webpack chunk loading method could be loaded later than react related chunks, that when client component is loaded first (e.g. `next/script`) and it triggers react loaded ealier than the overriding. Then the chunk could be encoded incorrectly.

Discussed with @gnoff and put this out as the 1st step solution to ensure the order. in the future we can try to get rid of the encoding by providing safer url

Fixes #57829
  • Loading branch information
huozhi authored Nov 2, 2023
1 parent c95ef23 commit 9af75d4
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 4 deletions.
2 changes: 1 addition & 1 deletion packages/next/src/client/app-next-dev.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
// TODO-APP: hydration warning

import './app-webpack'
import { appBootstrap } from './app-bootstrap'

appBootstrap(() => {
require('./app-webpack')
const { hydrate } = require('./app-index')
hydrate()
})
Expand Down
6 changes: 3 additions & 3 deletions packages/next/src/client/app-next.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
// This import must go first because it needs to patch webpack chunk loading
// before React patches chunk loading.
import './app-webpack'
import { appBootstrap } from './app-bootstrap'

appBootstrap(() => {
// This import must go first because it needs to patch webpack chunk loading
// before React patches chunk loading.
require('./app-webpack')
const { hydrate } = require('./app-index')
// Include app-router and layout-router in the main chunk
require('next/dist/client/components/app-router')
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// Add error.js to let this file be added to the flight manifest,
// and get url encoded, and load through react RSC renderer.

'use client'

export default function Error() {
return <div>My Error</div>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default function Page({ params }) {
return <div id="dynamic-gsp-content">{'slug:' + params.slug}</div>
}

export function generateStaticParams() {
return [{ slug: '1' }, { slug: '2' }, { slug: '3' }]
}
23 changes: 23 additions & 0 deletions test/e2e/app-dir/navigation/app/router/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
'use client'

import Script from 'next/script'
import { useRouter } from 'next/navigation'

export default function Page() {
const router = useRouter()
return (
<div>
<button
id="dynamic-link"
onClick={() => router.push('/router/dynamic-gsp/1/')}
>
Test routing
</button>
{/* adding a script here to make sure app internals might execute earlier */}
<Script
src="https://connect.facebook.net/en_US/sdk.js"
strategy="beforeInteractive"
/>
</div>
)
}
13 changes: 13 additions & 0 deletions test/e2e/app-dir/navigation/navigation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,19 @@ createNextDescribe(
}
}
})

it('should load chunks correctly without double encoding of url', async () => {
const browser = await next.browser('/router')

await browser
.elementByCss('#dynamic-link')
.click()
.waitForElementByCss('#dynamic-gsp-content')

expect(await browser.elementByCss('#dynamic-gsp-content').text()).toBe(
'slug:1'
)
})
})

describe('SEO', () => {
Expand Down

0 comments on commit 9af75d4

Please sign in to comment.