From 3647d7344d0b5991183e723ae7774266ad1bb1bf Mon Sep 17 00:00:00 2001 From: Hassan El Mghari Date: Fri, 2 Sep 2022 11:49:45 -0400 Subject: [PATCH 01/11] Updating the Next.js Logo (#40181) Updating the Next.js Logo, would love a quick merge --- packages/next/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/README.md b/packages/next/README.md index dd075a57575b..a85ea7acb225 100644 --- a/packages/next/README.md +++ b/packages/next/README.md @@ -1,6 +1,6 @@

- +

Next.js

From 885defd442fe2b5c5e75c4bf7e0c63cd1e1dfce5 Mon Sep 17 00:00:00 2001 From: Yusuke Hayashi Date: Sat, 3 Sep 2022 03:20:22 +0900 Subject: [PATCH 02/11] Fix typo in error/middleware-upgrade-guide.md (#40176) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No Reponse Body -> No Response Body スクリーンショット 2022-09-02 16 40 04 --- errors/middleware-upgrade-guide.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/errors/middleware-upgrade-guide.md b/errors/middleware-upgrade-guide.md index 8c04cc6c4f3b..94af0bceeae5 100644 --- a/errors/middleware-upgrade-guide.md +++ b/errors/middleware-upgrade-guide.md @@ -388,4 +388,4 @@ Prior to Next.js `v12.2`, Middleware was not executed for `_next` requests. For cases where Middleware is used for authorization, you should migrate to use `rewrite`/`redirect` to Pages that show an authorization error, login forms, or to an API Route. -See [No Reponse Body](#no-response-body) for an example of how to migrate to use `rewrite`/`redirect`. +See [No Response Body](#no-response-body) for an example of how to migrate to use `rewrite`/`redirect`. From f9706e036579b5b2a79cf2e39f9f57203b056e55 Mon Sep 17 00:00:00 2001 From: Sukka Date: Sat, 3 Sep 2022 06:25:44 +0800 Subject: [PATCH 03/11] next/script: simplify logic and update tests (#40026) The PR is the first step toward fixing #40025. The PR makes the `script-loader` integration test run on both dev and production modes. Some existing test cases are skipped in dev mode because corresponding features are not strict mode resilient and thus will fail. They will be included in dev mode tests in the future. The PR also merges some duplicated logic in `next/script`, and adds a detailed comment about how `onReady` works. In the next PR, I will try to fix `onLoad` being called more than once under strict mode. Co-authored-by: Houssein Djirdeh --- packages/next/client/script.tsx | 62 +++- .../{strictmode => base}/next.config.js | 0 .../script-loader/base/pages/page10.js | 7 +- .../script-loader/strictmode/pages/onready.js | 31 -- .../script-loader/test/index.test.js | 301 +++++++++--------- 5 files changed, 205 insertions(+), 196 deletions(-) rename test/integration/script-loader/{strictmode => base}/next.config.js (100%) delete mode 100644 test/integration/script-loader/strictmode/pages/onready.js diff --git a/packages/next/client/script.tsx b/packages/next/client/script.tsx index c1adef2378bd..e9ebd6a022c0 100644 --- a/packages/next/client/script.tsx +++ b/packages/next/client/script.tsx @@ -1,4 +1,4 @@ -import React, { useEffect, useContext } from 'react' +import React, { useEffect, useContext, useRef } from 'react' import { ScriptHTMLAttributes } from 'react' import { HeadManagerContext } from '../shared/lib/head-manager-context' import { DOMAttributeNames } from './head-manager' @@ -57,21 +57,25 @@ const loadScript = (props: ScriptProps): void => { return } + /** Execute after the script first loaded */ + const afterLoad = () => { + // Run onReady for the first time after load event + if (onReady) { + onReady() + } + // add cacheKey to LoadCache when load successfully + LoadCache.add(cacheKey) + } + const el = document.createElement('script') const loadPromise = new Promise((resolve, reject) => { el.addEventListener('load', function (e) { - // add cacheKey to LoadCache when load successfully - LoadCache.add(cacheKey) - resolve() if (onLoad) { onLoad.call(this, e) } - // Run onReady for the first time after load event - if (onReady) { - onReady() - } + afterLoad() }) el.addEventListener('error', function (e) { reject(e) @@ -85,8 +89,7 @@ const loadScript = (props: ScriptProps): void => { if (dangerouslySetInnerHTML) { el.innerHTML = dangerouslySetInnerHTML.__html || '' - // add cacheKey to LoadCache for inline script - LoadCache.add(cacheKey) + afterLoad() } else if (children) { el.textContent = typeof children === 'string' @@ -95,8 +98,7 @@ const loadScript = (props: ScriptProps): void => { ? children.join('') : '' - // add cacheKey to LoadCache for inline script - LoadCache.add(cacheKey) + afterLoad() } else if (src) { el.src = src // do not add cacheKey into LoadCache for remote script here @@ -174,12 +176,42 @@ function Script(props: ScriptProps): JSX.Element | null { // Context is available only during SSR const { updateScripts, scripts, getIsSsr } = useContext(HeadManagerContext) + /** + * - First mount: + * 1. The useEffect for onReady executes + * 2. hasOnReadyEffectCalled.current is false, but the script hasn't loaded yet (not in LoadCache) + * onReady is skipped, set hasOnReadyEffectCalled.current to true + * 3. The useEffect for loadScript executes + * Once the script is loaded, the onReady will be called by then + * [If strict mode is enabled / is wrapped in component] + * 5. The useEffect for onReady executes again + * 6. hasOnReadyEffectCalled.current is true, so entire effect is skipped + * 7. The useEffect for loadScript executes again + * 8. The script is already loaded/loading, loadScript bails out + * + * - Second mount: + * 1. The useEffect for onReady executes + * 2. hasOnReadyEffectCalled.current is false, but the script has already loaded (found in LoadCache) + * onReady is called, set hasOnReadyEffectCalled.current to true + * 3. The useEffect for loadScript executes + * 4. The script is already loaded, loadScript bails out + * [If strict mode is enabled / is wrapped in component] + * 5. The useEffect for onReady executes again + * 6. hasOnReadyEffectCalled.current is true, so entire effect is skipped + * 7. The useEffect for loadScript executes again + * 8. The script is already loaded, loadScript will bail out + */ + const hasOnReadyEffectCalled = useRef(false) + useEffect(() => { const cacheKey = id || src + if (!hasOnReadyEffectCalled.current) { + // Run onReady if script has loaded before but component is re-mounted + if (onReady && cacheKey && LoadCache.has(cacheKey)) { + onReady() + } - // Run onReady if script has loaded before but component is re-mounted - if (onReady && cacheKey && LoadCache.has(cacheKey)) { - onReady() + hasOnReadyEffectCalled.current = true } }, [onReady, id, src]) diff --git a/test/integration/script-loader/strictmode/next.config.js b/test/integration/script-loader/base/next.config.js similarity index 100% rename from test/integration/script-loader/strictmode/next.config.js rename to test/integration/script-loader/base/next.config.js diff --git a/test/integration/script-loader/base/pages/page10.js b/test/integration/script-loader/base/pages/page10.js index dd10c2f6a1d1..e2fdde0e776f 100644 --- a/test/integration/script-loader/base/pages/page10.js +++ b/test/integration/script-loader/base/pages/page10.js @@ -1,11 +1,6 @@ import Script from 'next/script' import Link from 'next/link' -if (typeof window !== 'undefined') { - window.remoteScriptsOnReadyCalls ??= 0 - window.inlineScriptsOnReadyCalls ??= 0 -} - const Page = () => { return (
@@ -14,6 +9,7 @@ const Page = () => {