From e8d5de6b072009e592f7aa19d894462146d74e05 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Tue, 25 Oct 2022 00:53:13 -0700 Subject: [PATCH 1/2] Only import dev overlay for dev mode (#41771) * Import dev overlay for dev mode only * Remove other unused code ## Bug - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Errors have a helpful link attached, see `contributing.md` ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. - [ ] Errors have a helpful link attached, see `contributing.md` ## Documentation / Examples - [ ] Make sure the linting passes by running `pnpm lint` - [ ] The "examples guidelines" are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md) --- packages/next/build/entries.ts | 2 +- packages/next/build/webpack-config.ts | 4 ++-- .../build/webpack/plugins/flight-client-entry-plugin.ts | 5 ++--- .../next/build/webpack/plugins/flight-manifest-plugin.ts | 4 +--- packages/next/client/app-index.tsx | 4 +++- test/.stats-app/app/app-edge-ssr/page.js | 4 +--- test/.stats-app/next.config.js | 8 ++++++++ test/e2e/app-dir/back-button-download-bug/app/page.js | 4 ++-- .../(mpa-navigation)/dynamic-catchall/[...slug]/page.js | 8 ++++---- 9 files changed, 24 insertions(+), 19 deletions(-) diff --git a/packages/next/build/entries.ts b/packages/next/build/entries.ts index c43f1d9f8abc4..a1dafb64bf67d 100644 --- a/packages/next/build/entries.ts +++ b/packages/next/build/entries.ts @@ -521,7 +521,7 @@ export function finalizeEntrypoint({ name !== CLIENT_STATIC_FILES_RUNTIME_REACT_REFRESH ) { // TODO-APP: this is a temporary fix. @shuding is going to change the handling of server components - if (hasAppDir && entry.import.includes('flight')) { + if (hasAppDir && entry.import.includes('next-flight-client-entry-loader')) { return { dependOn: CLIENT_STATIC_FILES_RUNTIME_MAIN_APP, ...entry, diff --git a/packages/next/build/webpack-config.ts b/packages/next/build/webpack-config.ts index c4c8c3198ef58..6647d7dfa0222 100644 --- a/packages/next/build/webpack-config.ts +++ b/packages/next/build/webpack-config.ts @@ -1618,7 +1618,7 @@ export default async function getBaseWebpackConfig( }, module: { rules: [ - ...(hasAppDir && !isClient && !isEdgeServer + ...(hasAppDir && !isClient ? [ { issuerLayer: WEBPACK_LAYERS.server, @@ -1643,7 +1643,7 @@ export default async function getBaseWebpackConfig( // If missing the alias override here, the default alias will be used which aliases // react to the direct file path, not the package name. In that case the condition // will be ignored completely. - react: 'next/dist/compiled/react', + react: 'next/dist/compiled/react/react.shared-subset', 'react-dom$': 'next/dist/compiled/react-dom/server-rendering-stub', }, diff --git a/packages/next/build/webpack/plugins/flight-client-entry-plugin.ts b/packages/next/build/webpack/plugins/flight-client-entry-plugin.ts index 41c983490cdca..fd8a299dcd14e 100644 --- a/packages/next/build/webpack/plugins/flight-client-entry-plugin.ts +++ b/packages/next/build/webpack/plugins/flight-client-entry-plugin.ts @@ -231,7 +231,7 @@ export class FlightClientEntryPlugin { function collectModule(entryName: string, mod: any) { const resource = mod.resource - const modId = resource // compilation.chunkGraph.getModuleId(mod) + '' + const modId = resource if (modId) { if (regexCSS.test(modId)) { cssImportsForChunk[entryName].push(modId) @@ -361,9 +361,8 @@ export class FlightClientEntryPlugin { !rawRequest.startsWith(APP_DIR_ALIAS) const modRequest: string | undefined = isLocal - ? rawRequest // compilation.chunkGraph.getModuleId(mod) + '' + ? rawRequest : mod.resourceResolveData?.path + mod.resourceResolveData?.query - // console.log('modId:after', modRequest) // Ensure module is not walked again if it's already been visited if (!visitedBySegment[layoutOrPageRequest]) { diff --git a/packages/next/build/webpack/plugins/flight-manifest-plugin.ts b/packages/next/build/webpack/plugins/flight-manifest-plugin.ts index 81dc53caeaada..1e63e4a4dece0 100644 --- a/packages/next/build/webpack/plugins/flight-manifest-plugin.ts +++ b/packages/next/build/webpack/plugins/flight-manifest-plugin.ts @@ -186,10 +186,8 @@ export class FlightManifestPlugin { context, mod.resourceResolveData?.path || resource ) - // if (resource.includes('script')) - // console.log('ssrNamedModuleId', ssrNamedModuleId, modId) + if (!ssrNamedModuleId.startsWith('.')) - // TODO use getModuleId instead ssrNamedModuleId = `./${ssrNamedModuleId.replace(/\\/g, '/')}` if (isCSSModule) { diff --git a/packages/next/client/app-index.tsx b/packages/next/client/app-index.tsx index 998b71af5bb23..fe45a3eacb350 100644 --- a/packages/next/client/app-index.tsx +++ b/packages/next/client/app-index.tsx @@ -7,7 +7,6 @@ import { createFromReadableStream } from 'next/dist/compiled/react-server-dom-we import measureWebVitals from './performance-relayer' import { HeadManagerContext } from '../shared/lib/head-manager-context' -import HotReload from './components/react-dev-overlay/hot-reloader-client' import { GlobalLayoutRouterContext } from '../shared/lib/app-router-context' /// @@ -178,6 +177,9 @@ export function hydrate() { if (process.env.NODE_ENV !== 'production') { const rootLayoutMissingTagsError = (self as any) .__next_root_layout_missing_tags_error + const HotReload: typeof import('./components/react-dev-overlay/hot-reloader-client').default = + require('./components/react-dev-overlay/hot-reloader-client') + .default as typeof import('./components/react-dev-overlay/hot-reloader-client').default // Don't try to hydrate if root layout is missing required tags, render error instead if (rootLayoutMissingTagsError) { diff --git a/test/.stats-app/app/app-edge-ssr/page.js b/test/.stats-app/app/app-edge-ssr/page.js index 20e49c40a12cc..8151cdff9bd97 100644 --- a/test/.stats-app/app/app-edge-ssr/page.js +++ b/test/.stats-app/app/app-edge-ssr/page.js @@ -2,6 +2,4 @@ export default function page() { return 'edge-ssr' } -export const config = { - runtime: 'experimental-edge', -} +export const runtime = 'experimental-edge' diff --git a/test/.stats-app/next.config.js b/test/.stats-app/next.config.js index cfa3ac3d7aa94..9544563de6496 100644 --- a/test/.stats-app/next.config.js +++ b/test/.stats-app/next.config.js @@ -3,3 +3,11 @@ module.exports = { appDir: true, }, } + +// For development: analyze the bundled chunks for stats app +if (process.env.ANALYZE) { + const withBundleAnalyzer = require('@next/bundle-analyzer')({ + enabled: true, + }) + module.exports = withBundleAnalyzer(module.exports) +} diff --git a/test/e2e/app-dir/back-button-download-bug/app/page.js b/test/e2e/app-dir/back-button-download-bug/app/page.js index 8b27c635f91d4..bbcdb0a8d9f3a 100644 --- a/test/e2e/app-dir/back-button-download-bug/app/page.js +++ b/test/e2e/app-dir/back-button-download-bug/app/page.js @@ -4,8 +4,8 @@ export default function Home() { return ( <>

Home!

- - To post 1 + + To post 1 To / diff --git a/test/e2e/app-dir/root-layout/app/(mpa-navigation)/dynamic-catchall/[...slug]/page.js b/test/e2e/app-dir/root-layout/app/(mpa-navigation)/dynamic-catchall/[...slug]/page.js index a4a02806a555e..5dece6d1e3e8d 100644 --- a/test/e2e/app-dir/root-layout/app/(mpa-navigation)/dynamic-catchall/[...slug]/page.js +++ b/test/e2e/app-dir/root-layout/app/(mpa-navigation)/dynamic-catchall/[...slug]/page.js @@ -4,11 +4,11 @@ export default function Page({ params }) { const nextUrl = [...params.slug, 'slug'] return ( <> - - To next url + + To next url - - To next url + + To next url

catchall {params.slug.join(' ')} From 67c802ac1746d74dee7dfdc248657c3c9d2c730b Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Tue, 25 Oct 2022 01:09:26 -0700 Subject: [PATCH 2/2] Add initial head handling in app (#41607) ## Bug - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Errors have a helpful link attached, see `contributing.md` ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. - [ ] Errors have a helpful link attached, see `contributing.md` ## Documentation / Examples - [ ] Make sure the linting passes by running `pnpm lint` - [ ] The "examples guidelines" are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md) --- .../build/webpack/loaders/next-app-loader.ts | 1 + packages/next/server/app-render.tsx | 28 ++++++ .../app-dir/back-button-download-bug.test.ts | 3 +- test/e2e/app-dir/head.test.ts | 97 +++++++++++++++++++ test/e2e/app-dir/head/app/blog/[slug]/head.js | 8 ++ test/e2e/app-dir/head/app/blog/[slug]/page.js | 13 +++ test/e2e/app-dir/head/app/blog/about/page.js | 13 +++ test/e2e/app-dir/head/app/blog/head.js | 10 ++ test/e2e/app-dir/head/app/blog/layout.js | 8 ++ test/e2e/app-dir/head/app/blog/page.js | 13 +++ test/e2e/app-dir/head/app/head.js | 9 ++ test/e2e/app-dir/head/app/layout.js | 10 ++ test/e2e/app-dir/head/app/page.js | 24 +++++ test/e2e/app-dir/head/next.config.js | 17 ++++ test/e2e/app-dir/head/public/another.js | 4 + test/e2e/app-dir/head/public/hello.js | 4 + test/e2e/app-dir/head/public/hello1.js | 4 + test/e2e/app-dir/head/public/hello2.js | 4 + test/e2e/app-dir/head/public/hello3.js | 4 + 19 files changed, 273 insertions(+), 1 deletion(-) create mode 100644 test/e2e/app-dir/head.test.ts create mode 100644 test/e2e/app-dir/head/app/blog/[slug]/head.js create mode 100644 test/e2e/app-dir/head/app/blog/[slug]/page.js create mode 100644 test/e2e/app-dir/head/app/blog/about/page.js create mode 100644 test/e2e/app-dir/head/app/blog/head.js create mode 100644 test/e2e/app-dir/head/app/blog/layout.js create mode 100644 test/e2e/app-dir/head/app/blog/page.js create mode 100644 test/e2e/app-dir/head/app/head.js create mode 100644 test/e2e/app-dir/head/app/layout.js create mode 100644 test/e2e/app-dir/head/app/page.js create mode 100644 test/e2e/app-dir/head/next.config.js create mode 100644 test/e2e/app-dir/head/public/another.js create mode 100644 test/e2e/app-dir/head/public/hello.js create mode 100644 test/e2e/app-dir/head/public/hello1.js create mode 100644 test/e2e/app-dir/head/public/hello2.js create mode 100644 test/e2e/app-dir/head/public/hello3.js diff --git a/packages/next/build/webpack/loaders/next-app-loader.ts b/packages/next/build/webpack/loaders/next-app-loader.ts index 93df9c1d85147..81a5238770788 100644 --- a/packages/next/build/webpack/loaders/next-app-loader.ts +++ b/packages/next/build/webpack/loaders/next-app-loader.ts @@ -13,6 +13,7 @@ export const FILE_TYPES = { template: 'template', error: 'error', loading: 'loading', + head: 'head', 'not-found': 'not-found', } as const diff --git a/packages/next/server/app-render.tsx b/packages/next/server/app-render.tsx index ed9c30292463c..63cc14cf86a58 100644 --- a/packages/next/server/app-render.tsx +++ b/packages/next/server/app-render.tsx @@ -912,6 +912,7 @@ export async function renderToHTMLOrFlight( template, error, loading, + head, page, 'not-found': notFound, }, @@ -919,12 +920,19 @@ export async function renderToHTMLOrFlight( parentParams, firstItem, rootLayoutIncluded, + collectedHeads = [], }: { createSegmentPath: CreateSegmentPath loaderTree: LoaderTree parentParams: { [key: string]: any } rootLayoutIncluded?: boolean firstItem?: boolean + collectedHeads?: Array< + (ctx: { + params?: Record + searchParams?: Record + }) => Promise + > }): Promise<{ Component: React.ComponentType }> => { // TODO-APP: enable stylesheet per layout/page const stylesheets: string[] = layoutOrPagePath @@ -948,6 +956,7 @@ export async function renderToHTMLOrFlight( : React.Fragment const ErrorComponent = error ? await interopDefault(error()) : undefined const Loading = loading ? await interopDefault(loading()) : undefined + const Head = head ? await interopDefault(head()) : undefined const isLayout = typeof layout !== 'undefined' const isPage = typeof page !== 'undefined' const layoutOrPageMod = isLayout @@ -1053,6 +1062,17 @@ export async function renderToHTMLOrFlight( // Resolve the segment param const actualSegment = segmentParam ? segmentParam.treeSegment : segment + // collect head pieces + if (typeof Head === 'function') { + collectedHeads.push(() => + Head({ + params: currentParams, + // TODO-APP: allow searchParams? + // ...(isPage ? { searchParams: query } : {}), + }) + ) + } + // This happens outside of rendering in order to eagerly kick off data fetching for layouts / the page further down const parallelRouteMap = await Promise.all( Object.keys(parallelRoutes).map( @@ -1102,6 +1122,7 @@ export async function renderToHTMLOrFlight( loaderTree: parallelRoutes[parallelRouteKey], parentParams: currentParams, rootLayoutIncluded: rootLayoutIncludedAtThisLevelOrAbove, + collectedHeads, }) const childProp: ChildProp = { @@ -1160,6 +1181,12 @@ export async function renderToHTMLOrFlight( // Add extra cache busting (DEV only) for https://github.com/vercel/next.js/issues/5860 // See also https://bugs.webkit.org/show_bug.cgi?id=187726 const cacheBustingUrlSuffix = dev ? `?ts=${Date.now()}` : '' + let HeadTags + if (rootLayoutAtThisLevel) { + // TODO: iterate HeadTag children and add a data-path attribute + // so that we can remove elements on client-transition + HeadTags = collectedHeads[collectedHeads.length - 1] as any + } return ( <> @@ -1200,6 +1227,7 @@ export async function renderToHTMLOrFlight( // Query is only provided to page {...(isPage ? { searchParams: query } : {})} /> + {HeadTags ? : null} ) }, diff --git a/test/e2e/app-dir/back-button-download-bug.test.ts b/test/e2e/app-dir/back-button-download-bug.test.ts index 707a6062bc07b..ad8273c2568cf 100644 --- a/test/e2e/app-dir/back-button-download-bug.test.ts +++ b/test/e2e/app-dir/back-button-download-bug.test.ts @@ -3,7 +3,8 @@ import { NextInstance } from 'test/lib/next-modes/base' import path from 'path' import webdriver from 'next-webdriver' -describe('app-dir back button download bug', () => { +// TODO-APP: fix test as it's failing randomly +describe.skip('app-dir back button download bug', () => { if ((global as any).isNextDeploy) { it('should skip next deploy for now', () => {}) return diff --git a/test/e2e/app-dir/head.test.ts b/test/e2e/app-dir/head.test.ts new file mode 100644 index 0000000000000..1b70def957218 --- /dev/null +++ b/test/e2e/app-dir/head.test.ts @@ -0,0 +1,97 @@ +import path from 'path' +import cheerio from 'cheerio' +import { createNext, FileRef } from 'e2e-utils' +import { NextInstance } from 'test/lib/next-modes/base' +import { renderViaHTTP } from 'next-test-utils' + +describe('app dir head', () => { + if ((global as any).isNextDeploy) { + it('should skip next deploy for now', () => {}) + return + } + + if (process.env.NEXT_TEST_REACT_VERSION === '^17') { + it('should skip for react v17', () => {}) + return + } + let next: NextInstance + + function runTests() { + beforeAll(async () => { + next = await createNext({ + files: new FileRef(path.join(__dirname, 'head')), + dependencies: { + react: 'experimental', + 'react-dom': 'experimental', + }, + skipStart: true, + }) + + await next.start() + }) + afterAll(() => next.destroy()) + + it('should use head from index page', async () => { + const html = await renderViaHTTP(next.url, '/') + const $ = cheerio.load(html) + const headTags = $('head').children().toArray() + + expect(headTags.find((el) => el.attribs.src === '/hello.js')).toBeTruthy() + expect( + headTags.find((el) => el.attribs.src === '/another.js') + ).toBeTruthy() + }) + + it('should use correct head for /blog', async () => { + const html = await renderViaHTTP(next.url, '/blog') + const $ = cheerio.load(html) + const headTags = $('head').children().toArray() + + expect(headTags.find((el) => el.attribs.src === '/hello3.js')).toBeFalsy() + expect( + headTags.find((el) => el.attribs.src === '/hello1.js') + ).toBeTruthy() + expect( + headTags.find((el) => el.attribs.src === '/hello2.js') + ).toBeTruthy() + expect( + headTags.find((el) => el.attribs.src === '/another.js') + ).toBeTruthy() + }) + + it('should use head from layout when not on page', async () => { + const html = await renderViaHTTP(next.url, '/blog/about') + const $ = cheerio.load(html) + const headTags = $('head').children().toArray() + + expect( + headTags.find((el) => el.attribs.src === '/hello1.js') + ).toBeTruthy() + expect( + headTags.find((el) => el.attribs.src === '/hello2.js') + ).toBeTruthy() + expect( + headTags.find((el) => el.attribs.src === '/another.js') + ).toBeTruthy() + }) + + it('should pass params to head for dynamic path', async () => { + const html = await renderViaHTTP(next.url, '/blog/post-1') + const $ = cheerio.load(html) + const headTags = $('head').children().toArray() + + expect( + headTags.find( + (el) => + el.attribs.src === '/hello3.js' && + el.attribs['data-slug'] === 'post-1' + ) + ).toBeTruthy() + expect( + headTags.find((el) => el.attribs.src === '/another.js') + ).toBeTruthy() + }) + } + + runTests() +}) diff --git a/test/e2e/app-dir/head/app/blog/[slug]/head.js b/test/e2e/app-dir/head/app/blog/[slug]/head.js new file mode 100644 index 0000000000000..8829dd802c2fe --- /dev/null +++ b/test/e2e/app-dir/head/app/blog/[slug]/head.js @@ -0,0 +1,8 @@ +export default async function Head({ params }) { + return ( + <> +