Turbopack: Implement HMR for module-scoped environment variable chang…

…es (#68209) While we currently refetch RSC updates on changes to the environment (e.g. `.env` files), we never cleared the require cache to re-evaluate RSCs with the new environment. This implements that. Test Plan: Added tests. `TURBOPACK=1 pnpm test-dev test/development/app-hmr/hmr.test.ts`
vercel · Jul 30, 2024 · 2c8736a · 2c8736a
1 parent c66f7ed
commit 2c8736a
Show file tree

Hide file tree

Showing 4 changed files with 94 additions and 109 deletions.
diff --git a/packages/next/src/server/dev/hot-reloader-turbopack.ts b/packages/next/src/server/dev/hot-reloader-turbopack.ts
@@ -157,6 +157,7 @@ export async function createHotReloaderTurbopack(
   opts.onCleanup(() => project.onExit())
   const entrypointsSubscription = project.entrypointsSubscribe()
 
+  const currentWrittenEntrypoints: Map<EntryKey, WrittenEndpoint> = new Map()
   const currentEntrypoints: Entrypoints = {
     global: {
       app: undefined,
@@ -193,35 +194,47 @@ export async function createHotReloaderTurbopack(
 
   function clearRequireCache(
     key: EntryKey,
-    writtenEndpoint: WrittenEndpoint
+    writtenEndpoint: WrittenEndpoint,
+    {
+      force,
+    }: {
+      // Always clear the cache, don't check if files have changed
+      force?: boolean
+    } = {}
   ): void {
-    // Figure out if the server files have changed
-    let hasChange = false
-    for (const { path, contentHash } of writtenEndpoint.serverPaths) {
-      // We ignore source maps
-      if (path.endsWith('.map')) continue
-      const localKey = `${key}:${path}`
-      const localHash = serverPathState.get(localKey)
-      const globalHash = serverPathState.get(path)
-      if (
-        (localHash && localHash !== contentHash) ||
-        (globalHash && globalHash !== contentHash)
-      ) {
-        hasChange = true
-        serverPathState.set(key, contentHash)
+    if (force) {
+      for (const { path, contentHash } of writtenEndpoint.serverPaths) {
         serverPathState.set(path, contentHash)
-      } else {
-        if (!localHash) {
+      }
+    } else {
+      // Figure out if the server files have changed
+      let hasChange = false
+      for (const { path, contentHash } of writtenEndpoint.serverPaths) {
+        // We ignore source maps
+        if (path.endsWith('.map')) continue
+        const localKey = `${key}:${path}`
+        const localHash = serverPathState.get(localKey)
+        const globalHash = serverPathState.get(path)
+        if (
+          (localHash && localHash !== contentHash) ||
+          (globalHash && globalHash !== contentHash)
+        ) {
+          hasChange = true
           serverPathState.set(key, contentHash)
-        }
-        if (!globalHash) {
           serverPathState.set(path, contentHash)
+        } else {
+          if (!localHash) {
+            serverPathState.set(key, contentHash)
+          }
+          if (!globalHash) {
+            serverPathState.set(path, contentHash)
+          }
         }
       }
-    }
 
-    if (!hasChange) {
-      return
+      if (!hasChange) {
+        return
+      }
     }
 
     const hasAppPaths = writtenEndpoint.serverPaths.some(({ path: p }) =>
@@ -477,6 +490,7 @@ export async function createHotReloaderTurbopack(
 
           hooks: {
             handleWrittenEndpoint: (id, result) => {
+              currentWrittenEntrypoints.set(id, result)
               clearRequireCache(id, result)
             },
             propagateServerField: propagateServerField.bind(null, opts),
@@ -754,6 +768,10 @@ export async function createHotReloaderTurbopack(
       reloadAfterInvalidation,
     }) {
       if (reloadAfterInvalidation) {
+        for (const [key, entrypoint] of currentWrittenEntrypoints) {
+          clearRequireCache(key, entrypoint, { force: true })
+        }
+
         await clearAllModuleContexts()
         this.send({
           action: HMR_ACTIONS_SENT_TO_BROWSER.SERVER_COMPONENT_CHANGES,
@@ -822,6 +840,7 @@ export async function createHotReloaderTurbopack(
               subscribeToChanges,
               handleWrittenEndpoint: (id, result) => {
                 clearRequireCache(id, result)
+                currentWrittenEntrypoints.set(id, result)
                 assetMapper.setPathsForKey(id, result.clientPaths)
               },
             },
@@ -879,6 +898,7 @@ export async function createHotReloaderTurbopack(
           hooks: {
             subscribeToChanges,
             handleWrittenEndpoint: (id, result) => {
+              currentWrittenEntrypoints.set(id, result)
               clearRequireCache(id, result)
               assetMapper.setPathsForKey(id, result.clientPaths)
             },

diff --git a/test/development/app-hmr/app/env/edge-module-var/page.jsx b/test/development/app-hmr/app/env/edge-module-var/page.jsx
@@ -0,0 +1,7 @@
+const MY_DEVICE = process.env.MY_DEVICE?.slice()
+
+export default function Page() {
+  return <p>{MY_DEVICE}</p>
+}
+
+export const runtime = 'edge'
diff --git a/test/development/app-hmr/app/env/node-module-var/page.jsx b/test/development/app-hmr/app/env/node-module-var/page.jsx
@@ -0,0 +1,5 @@
+const MY_DEVICE = process.env.MY_DEVICE?.slice()
+
+export default function Page() {
+  return <p>{MY_DEVICE}</p>
+}
diff --git a/test/development/app-hmr/hmr.test.ts b/test/development/app-hmr/hmr.test.ts
@@ -140,107 +140,60 @@ describe(`app-dir-hmr`, () => {
       }
     })
 
-    it('should update server components pages when env files is changed (nodejs)', async () => {
-      const browser = await next.browser('/env/node')
-      expect(await browser.elementByCss('p').text()).toBe('mac')
-      await next.patchFile(envFile, 'MY_DEVICE="ipad"')
-
-      const logs = await browser.log()
-      await retry(async () => {
-        expect(logs).toEqual(
-          expect.arrayContaining([
-            expect.objectContaining({
-              message: '[Fast Refresh] rebuilding',
-              source: 'log',
-            }),
-          ])
-        )
-      })
+    it.each(['node', 'node-module-var', 'edge', 'edge-module-var'])(
+      'should update server components pages when env files is changed (%s)',
+      async (page) => {
+        const browser = await next.browser(`/env/${page}`)
+        expect(await browser.elementByCss('p').text()).toBe('mac')
+        await next.patchFile(envFile, 'MY_DEVICE="ipad"')
 
-      try {
+        const logs = await browser.log()
         await retry(async () => {
-          expect(await browser.elementByCss('p').text()).toBe('ipad')
-        })
-
-        if (process.env.TURBOPACK) {
-          // FIXME: Turbopack should have matching "done in" for each "rebuilding"
-          expect(logs).not.toEqual(
-            expect.arrayContaining([
-              expect.objectContaining({
-                message: expect.stringContaining('[Fast Refresh] done in'),
-                source: 'log',
-              }),
-            ])
-          )
-        } else {
           expect(logs).toEqual(
             expect.arrayContaining([
               expect.objectContaining({
-                message: expect.stringContaining('[Fast Refresh] done in'),
+                message: '[Fast Refresh] rebuilding',
                 source: 'log',
               }),
             ])
           )
-        }
-      } finally {
-        // TOOD: use sandbox instead
-        await next.patchFile(envFile, 'MY_DEVICE="mac"')
-        await retry(async () => {
-          expect(await browser.elementByCss('p').text()).toBe('mac')
         })
-      }
-    })
 
-    it('should update server components pages when env files is changed (edge)', async () => {
-      const browser = await next.browser('/env/edge')
-      expect(await browser.elementByCss('p').text()).toBe('mac')
-      await next.patchFile(envFile, 'MY_DEVICE="ipad"')
-
-      const logs = await browser.log()
-      await retry(async () => {
-        expect(logs).toEqual(
-          expect.arrayContaining([
-            expect.objectContaining({
-              message: '[Fast Refresh] rebuilding',
-              source: 'log',
-            }),
-          ])
-        )
-      })
-
-      try {
-        await retry(async () => {
-          expect(await browser.elementByCss('p').text()).toBe('ipad')
-        })
+        try {
+          await retry(async () => {
+            expect(await browser.elementByCss('p').text()).toBe('ipad')
+          })
 
-        if (process.env.TURBOPACK) {
-          // FIXME: Turbopack should have matching "done in" for each "rebuilding"
-          expect(logs).not.toEqual(
-            expect.arrayContaining([
-              expect.objectContaining({
-                message: expect.stringContaining('[Fast Refresh] done in'),
-                source: 'log',
-              }),
-            ])
-          )
-        } else {
-          expect(logs).toEqual(
-            expect.arrayContaining([
-              expect.objectContaining({
-                message: expect.stringContaining('[Fast Refresh] done in'),
-                source: 'log',
-              }),
-            ])
-          )
+          if (process.env.TURBOPACK) {
+            // FIXME: Turbopack should have matching "done in" for each "rebuilding"
+            expect(logs).not.toEqual(
+              expect.arrayContaining([
+                expect.objectContaining({
+                  message: expect.stringContaining('[Fast Refresh] done in'),
+                  source: 'log',
+                }),
+              ])
+            )
+          } else {
+            expect(logs).toEqual(
+              expect.arrayContaining([
+                expect.objectContaining({
+                  message: expect.stringContaining('[Fast Refresh] done in'),
+                  source: 'log',
+                }),
+              ])
+            )
+          }
+        } finally {
+          // TOOD: use sandbox instead
+          await next.patchFile(envFile, 'MY_DEVICE="mac"')
+          await retry(async () => {
+            console.log('checking...', await browser.elementByCss('p').text())
+            expect(await browser.elementByCss('p').text()).toBe('mac')
+          })
         }
-      } finally {
-        // TOOD: use sandbox instead
-        await next.patchFile(envFile, 'MY_DEVICE="mac"')
-        await retry(async () => {
-          expect(await browser.elementByCss('p').text()).toBe('mac')
-        })
       }
-    })
+    )
 
     it('should have no unexpected action error for hmr', async () => {
       expect(next.cliOutput).not.toContain('Unexpected action')