From 24034ec95765bce113c7f5b55282a414a901cfe4 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Tue, 28 May 2019 17:32:18 -0700 Subject: [PATCH] Add amp.canonicalBase option to set absolute URL (#7262) * Add canonicalBase config to allow setting absolute path for canonical link * Make sure canonicalBase is set for export and serverless * Move canonicalBase to amp.canonicalBase * Update tests with canonicalBase config * Update tests * run lint-fix * Fix canonicalBase config parsing * Fix canonicalBase during export * Update amphtml tests --- packages/next-server/lib/utils.ts | 1 + packages/next-server/server/config.ts | 11 +++++++++++ packages/next-server/server/next-server.ts | 5 ++++- packages/next-server/server/render.tsx | 3 +++ packages/next/build/entries.ts | 3 ++- .../build/webpack/loaders/next-serverless-loader.ts | 5 ++++- packages/next/export/index.js | 3 ++- packages/next/pages/_document.tsx | 7 ++++--- test/integration/amphtml/next.config.js | 3 +++ test/integration/amphtml/test/index.test.js | 13 ++++++++----- 10 files changed, 42 insertions(+), 12 deletions(-) diff --git a/packages/next-server/lib/utils.ts b/packages/next-server/lib/utils.ts index c65c7a5afd..d7ac463e79 100644 --- a/packages/next-server/lib/utils.ts +++ b/packages/next-server/lib/utils.ts @@ -134,6 +134,7 @@ export type DocumentProps = DocumentInitialProps & { files: string[] dynamicImports: ManifestItem[] assetPrefix?: string, + canonicalBase: string, } /** diff --git a/packages/next-server/server/config.ts b/packages/next-server/server/config.ts index 00bf360caa..4c646044e0 100644 --- a/packages/next-server/server/config.ts +++ b/packages/next-server/server/config.ts @@ -21,6 +21,9 @@ const defaultConfig: {[key: string]: any} = { maxInactiveAge: 60 * 1000, pagesBufferLength: 2, }, + amp: { + canonicalBase: '', + }, experimental: { cpus: Math.max( 1, @@ -80,6 +83,14 @@ export default function loadConfig(phase: string, dir: string, customConfig: any if (userConfig.target && !targets.includes(userConfig.target)) { throw new Error(`Specified target is invalid. Provided: "${userConfig.target}" should be one of ${targets.join(', ')}`) } + + if (userConfig.amp && userConfig.amp.canonicalBase) { + const { canonicalBase } = userConfig.amp || {} as any + userConfig.amp = userConfig.amp || {} + userConfig.amp.canonicalBase = (canonicalBase.endsWith('/') + ? canonicalBase.slice(0, -1) : canonicalBase) || '' + } + return assignDefaults({ configOrigin: CONFIG_FILE, ...userConfig }) } diff --git a/packages/next-server/server/next-server.ts b/packages/next-server/server/next-server.ts index 8b42ca5e13..b650ddde48 100644 --- a/packages/next-server/server/next-server.ts +++ b/packages/next-server/server/next-server.ts @@ -55,7 +55,8 @@ export default class Server { buildId: string generateEtags: boolean runtimeConfig?: { [key: string]: any } - assetPrefix?: string + assetPrefix?: string, + canonicalBase: string, autoExport: boolean dev?: boolean, } @@ -87,9 +88,11 @@ export default class Server { } = this.nextConfig this.buildId = this.readBuildId() + this.renderOpts = { ampBindInitData: this.nextConfig.experimental.ampBindInitData, poweredByHeader: this.nextConfig.poweredByHeader, + canonicalBase: this.nextConfig.amp.canonicalBase, autoExport: this.nextConfig.experimental.autoExport, staticMarkup, buildId: this.buildId, diff --git a/packages/next-server/server/render.tsx b/packages/next-server/server/render.tsx index 44bc2b6a0d..8d003c661f 100644 --- a/packages/next-server/server/render.tsx +++ b/packages/next-server/server/render.tsx @@ -113,6 +113,7 @@ type RenderOpts = { ampBindInitData: boolean staticMarkup: boolean buildId: string + canonicalBase: string dynamicBuildId?: boolean runtimeConfig?: { [key: string]: any } dangerousAsPath: string @@ -144,6 +145,7 @@ function renderDocument( pathname, query, buildId, + canonicalBase, dynamicBuildId = false, assetPrefix, runtimeConfig, @@ -196,6 +198,7 @@ function renderDocument( err: err ? serializeError(dev, err) : undefined, // Error if one happened, otherwise don't sent in the resulting HTML }} dangerousAsPath={dangerousAsPath} + canonicalBase={canonicalBase} ampPath={ampPath} amphtml={amphtml} hasAmp={hasAmp} diff --git a/packages/next/build/entries.ts b/packages/next/build/entries.ts index cbb833e5a0..116f1f256a 100644 --- a/packages/next/build/entries.ts +++ b/packages/next/build/entries.ts @@ -44,7 +44,8 @@ export function createEntrypoints(pages: PagesMapping, target: 'server'|'serverl assetPrefix: config.assetPrefix, generateEtags: config.generateEtags, ampBindInitData: config.experimental.ampBindInitData, - dynamicBuildId + canonicalBase: config.canonicalBase, + dynamicBuildId, } Object.keys(pages).forEach((page) => { diff --git a/packages/next/build/webpack/loaders/next-serverless-loader.ts b/packages/next/build/webpack/loaders/next-serverless-loader.ts index 303a48698f..6840724d87 100644 --- a/packages/next/build/webpack/loaders/next-serverless-loader.ts +++ b/packages/next/build/webpack/loaders/next-serverless-loader.ts @@ -13,7 +13,8 @@ export type ServerlessLoaderQuery = { assetPrefix: string, ampBindInitData: boolean | string, generateEtags: string - dynamicBuildId?: string | boolean + dynamicBuildId?: string | boolean, + canonicalBase: string } const nextServerlessLoader: loader.Loader = function () { @@ -21,6 +22,7 @@ const nextServerlessLoader: loader.Loader = function () { distDir, absolutePagePath, page, + canonicalBase, assetPrefix, ampBindInitData, absoluteAppPath, @@ -54,6 +56,7 @@ const nextServerlessLoader: loader.Loader = function () { Document, buildManifest, reactLoadableManifest, + canonicalBase: "${canonicalBase}", buildId: "__NEXT_REPLACE__BUILD_ID__", dynamicBuildId: ${dynamicBuildId === true || dynamicBuildId === 'true'}, assetPrefix: "${assetPrefix}", diff --git a/packages/next/export/index.js b/packages/next/export/index.js index 90b75d7f20..e797b82742 100644 --- a/packages/next/export/index.js +++ b/packages/next/export/index.js @@ -91,7 +91,8 @@ export default async function (dir, options, configuration) { distDir, dev: false, staticMarkup: false, - hotReloader: null + hotReloader: null, + canonicalBase: (nextConfig.amp && nextConfig.amp.canonicalBase) || '' } const { serverRuntimeConfig, publicRuntimeConfig } = nextConfig diff --git a/packages/next/pages/_document.tsx b/packages/next/pages/_document.tsx index c7f305cfce..d7f487e625 100644 --- a/packages/next/pages/_document.tsx +++ b/packages/next/pages/_document.tsx @@ -191,6 +191,7 @@ export class Head extends Component { hasAmp, ampPath, assetPrefix, + canonicalBase, __NEXT_DATA__, dangerousAsPath, } = this.context._documentProps @@ -280,8 +281,8 @@ export class Head extends Component { name="viewport" content="width=device-width,minimum-scale=1,initial-scale=1" /> - - {isDirtyAmp && } + + {isDirtyAmp && } {/* https://www.ampproject.org/docs/fundamentals/optimize_amp#optimize-the-amp-runtime-loading */} { )} {!amphtml && ( <> - {hasAmp && } + {hasAmp && } {page !== '/_error' && ( { expect( () => accessSync(join(appDir, '.next/server/static', buildId, 'pages', pg + '.html')) ).not.toThrow() + expect( + () => accessSync(join(appDir, '.next/server/static', buildId, 'pages', pg + '.js')) + ).toThrow() } }) @@ -163,7 +166,7 @@ describe('AMP Usage', () => { $('link[rel=amphtml]') .first() .attr('href') - ).toBe('/use-amp-hook.amp') + ).toBe('http://localhost:1234/use-amp-hook.amp') }) it('should render link rel amphtml with existing query', async () => { @@ -179,7 +182,7 @@ describe('AMP Usage', () => { $('link[rel=canonical]') .first() .attr('href') - ).toBe('/use-amp-hook') + ).toBe('http://localhost:1234/use-amp-hook') }) it('should render a canonical regardless of amp-only status (implicit)', async () => { @@ -189,7 +192,7 @@ describe('AMP Usage', () => { $('link[rel=canonical]') .first() .attr('href') - ).toBe('/only-amp') + ).toBe('http://localhost:1234/only-amp') }) it('should render a canonical regardless of amp-only status (explicit)', async () => { @@ -200,7 +203,7 @@ describe('AMP Usage', () => { $('link[rel=canonical]') .first() .attr('href') - ).toBe('/only-amp') + ).toBe('http://localhost:1234/only-amp') }) it('should not render amphtml link tag with no AMP page', async () => { @@ -220,7 +223,7 @@ describe('AMP Usage', () => { $('link[rel=amphtml]') .first() .attr('href') - ).toBe('/only-amp.amp') + ).toBe('http://localhost:1234/only-amp.amp') }) it('should remove conflicting amp tags', async () => { -- GitLab