From 2ec7b97eeb42f4d63fe3f13a09d67c2048cfd975 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Tue, 15 Oct 2019 17:48:48 +0300 Subject: [PATCH 1/5] allow to remove amphtml from desktop page --- packages/next/pages/_document.tsx | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/next/pages/_document.tsx b/packages/next/pages/_document.tsx index ca723cc5eacee..985a8c1c54f75 100644 --- a/packages/next/pages/_document.tsx +++ b/packages/next/pages/_document.tsx @@ -303,7 +303,14 @@ export class Head extends Component< } return child }) - + head = React.Children.map(head || [], child => { + if (!child) return child + const { type, props } = child + if (type === 'link' && props.rel === 'amphtml') { + hasAmphtmlRel = true + } + return child + }) // try to parse styles from fragment for backwards compat const curStyles: React.ReactElement[] = Array.isArray(styles) ? (styles as React.ReactElement[]) From 1765b2141b4e5097465847d0bef6219d71ebd9c9 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Wed, 16 Oct 2019 00:53:57 +0300 Subject: [PATCH 2/5] only 1 link with amphtml rel --- test/integration/amphtml/test/index.test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/integration/amphtml/test/index.test.js b/test/integration/amphtml/test/index.test.js index 197b5128c6ad9..e9e45ad11e2dc 100644 --- a/test/integration/amphtml/test/index.test.js +++ b/test/integration/amphtml/test/index.test.js @@ -199,7 +199,9 @@ describe('AMP Usage', () => { it('should allow manually setting amphtml rel', async () => { const html = await renderViaHTTP(appPort, '/manual-rels') const $ = cheerio.load(html) + console.log($('link[rel=amphtml]').length) expect($('link[rel=amphtml]').attr('href')).toBe('/my-custom-amphtml') + expect($('link[rel=amphtml]')).toHaveLength(1) }) }) From e717cce3d32f01bde851fcb9cd8214532329f379 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Wed, 16 Oct 2019 15:02:59 +0300 Subject: [PATCH 3/5] clean up --- test/integration/amphtml/test/index.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/integration/amphtml/test/index.test.js b/test/integration/amphtml/test/index.test.js index e9e45ad11e2dc..bbbe78c127ce1 100644 --- a/test/integration/amphtml/test/index.test.js +++ b/test/integration/amphtml/test/index.test.js @@ -199,7 +199,6 @@ describe('AMP Usage', () => { it('should allow manually setting amphtml rel', async () => { const html = await renderViaHTTP(appPort, '/manual-rels') const $ = cheerio.load(html) - console.log($('link[rel=amphtml]').length) expect($('link[rel=amphtml]').attr('href')).toBe('/my-custom-amphtml') expect($('link[rel=amphtml]')).toHaveLength(1) }) From 2d837f49c2a359d84d5628bbc215b38e4949b00c Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Wed, 16 Oct 2019 15:03:47 +0300 Subject: [PATCH 4/5] remove checkup for amphtml link --- packages/next/pages/_document.tsx | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/packages/next/pages/_document.tsx b/packages/next/pages/_document.tsx index 985a8c1c54f75..c43ab17dcfe6e 100644 --- a/packages/next/pages/_document.tsx +++ b/packages/next/pages/_document.tsx @@ -271,8 +271,6 @@ export class Head extends Component< badProp = 'name="viewport"' } else if (type === 'link' && props.rel === 'canonical') { hasCanonicalRel = true - } else if (type === 'link' && props.rel === 'amphtml') { - hasAmphtmlRel = true } else if (type === 'script') { // only block if // 1. it has a src and isn't pointing to ampproject's CDN @@ -303,14 +301,17 @@ export class Head extends Component< } return child }) - head = React.Children.map(head || [], child => { - if (!child) return child - const { type, props } = child - if (type === 'link' && props.rel === 'amphtml') { - hasAmphtmlRel = true - } - return child - }) + // Handle non-amp logic + head = inAmpMode + ? head + : React.Children.map(head || [], child => { + if (!child) return child + const { type, props } = child + if (type === 'link' && props.rel === 'amphtml') { + hasAmphtmlRel = true + } + return child + }) // try to parse styles from fragment for backwards compat const curStyles: React.ReactElement[] = Array.isArray(styles) ? (styles as React.ReactElement[]) From e0c87858e80941ccd7b4185fcb754363af67ab80 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Fri, 18 Oct 2019 10:10:52 -0500 Subject: [PATCH 5/5] Dedupe looping --- packages/next/pages/_document.tsx | 96 +++++++++++++++---------------- 1 file changed, 46 insertions(+), 50 deletions(-) diff --git a/packages/next/pages/_document.tsx b/packages/next/pages/_document.tsx index 444ec5312c1d2..2f7b144ff933e 100644 --- a/packages/next/pages/_document.tsx +++ b/packages/next/pages/_document.tsx @@ -273,58 +273,54 @@ export class Head extends Component< let hasCanonicalRel = false // show warning and remove conflicting amp head tags - head = !inAmpMode - ? head - : React.Children.map(head || [], child => { - if (!child) return child - const { type, props } = child - let badProp: string = '' - - if (type === 'meta' && props.name === 'viewport') { - badProp = 'name="viewport"' - } else if (type === 'link' && props.rel === 'canonical') { - hasCanonicalRel = true - } else if (type === 'script') { - // only block if - // 1. it has a src and isn't pointing to ampproject's CDN - // 2. it is using dangerouslySetInnerHTML without a type or - // a type of text/javascript - if ( - (props.src && props.src.indexOf('ampproject') < -1) || - (props.dangerouslySetInnerHTML && - (!props.type || props.type === 'text/javascript')) - ) { - badProp = ' { - badProp += ` ${prop}="${props[prop]}"` - }) - badProp += '/>' - } + head = React.Children.map(head || [], child => { + if (!child) return child + const { type, props } = child + + if (inAmpMode) { + let badProp: string = '' + + if (type === 'meta' && props.name === 'viewport') { + badProp = 'name="viewport"' + } else if (type === 'link' && props.rel === 'canonical') { + hasCanonicalRel = true + } else if (type === 'script') { + // only block if + // 1. it has a src and isn't pointing to ampproject's CDN + // 2. it is using dangerouslySetInnerHTML without a type or + // a type of text/javascript + if ( + (props.src && props.src.indexOf('ampproject') < -1) || + (props.dangerouslySetInnerHTML && + (!props.type || props.type === 'text/javascript')) + ) { + badProp = ' { + badProp += ` ${prop}="${props[prop]}"` + }) + badProp += '/>' } + } + + if (badProp) { + console.warn( + `Found conflicting amp tag "${ + child.type + }" with conflicting prop ${badProp} in ${ + __NEXT_DATA__.page + }. https://err.sh/next.js/conflicting-amp-tag` + ) + return null + } + } else { + // non-amp mode + if (type === 'link' && props.rel === 'amphtml') { + hasAmphtmlRel = true + } + } + return child + }) - if (badProp) { - console.warn( - `Found conflicting amp tag "${ - child.type - }" with conflicting prop ${badProp} in ${ - __NEXT_DATA__.page - }. https://err.sh/next.js/conflicting-amp-tag` - ) - return null - } - return child - }) - // Handle non-amp logic - head = inAmpMode - ? head - : React.Children.map(head || [], child => { - if (!child) return child - const { type, props } = child - if (type === 'link' && props.rel === 'amphtml') { - hasAmphtmlRel = true - } - return child - }) // try to parse styles from fragment for backwards compat const curStyles: React.ReactElement[] = Array.isArray(styles) ? (styles as React.ReactElement[])