Fix infinite invalidations loop in app dir (#46526)Co-authored-by: ko…

…diakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> This fixes a case where depending on the timings of compilers being done could cause an infinite invalidation loop. **Before** https://user-images.githubusercontent.com/22380829/221786587-1a4cc6ab-f273-4191-92af-a57e9fff1261.mp4 **After** https://user-images.githubusercontent.com/22380829/221786611-f55c3da9-0201-40be-95a8-3ef1869d6a66.mp4 x-ref: [slack thread](https://vercel.slack.com/archives/C03KAR5DCKC/p1677441845988529?thread_ts=1677429424.151329&cid=C03KAR5DCKC) ## Bug - [x] Related issues linked using `fixes #number` - [x] Integration tests added - [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md) --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
vercel · Feb 28, 2023 · 619c76c · 619c76c
1 parent 5ff005f
commit 619c76c
Show file tree

Hide file tree

Showing 32 changed files with 599 additions and 7 deletions.
diff --git a/packages/next/src/server/dev/on-demand-entry-handler.ts b/packages/next/src/server/dev/on-demand-entry-handler.ts
@@ -215,18 +215,18 @@ class Invalidator {
         continue
       }
 
-      this.multiCompiler.compilers[COMPILER_INDEXES[key]].watching?.invalidate()
       this.building.add(key)
+      this.multiCompiler.compilers[COMPILER_INDEXES[key]].watching?.invalidate()
     }
   }
 
   public startBuilding(compilerKey: keyof typeof COMPILER_INDEXES) {
     this.building.add(compilerKey)
   }
 
-  public doneBuilding() {
+  public doneBuilding(compilerKeys: typeof COMPILER_KEYS = []) {
     const rebuild: typeof COMPILER_KEYS = []
-    for (const key of COMPILER_KEYS) {
+    for (const key of compilerKeys) {
       this.building.delete(key)
 
       if (this.rebuildAgain.has(key)) {
@@ -452,10 +452,15 @@ export function onDemandEntryHandler({
     return pagePaths
   }
 
+  for (const compiler of multiCompiler.compilers) {
+    compiler.hooks.done.tap('NextJsOnDemandEntries', () =>
+      getInvalidator().doneBuilding([
+        compiler.name as keyof typeof COMPILER_INDEXES,
+      ])
+    )
+  }
+
   multiCompiler.hooks.done.tap('NextJsOnDemandEntries', (multiStats) => {
-    if (invalidator.shouldRebuildAll()) {
-      return invalidator.doneBuilding()
-    }
     const [clientStats, serverStats, edgeServerStats] = multiStats.stats
     const root = !!appDir
     const pagePaths = [
@@ -492,7 +497,7 @@ export function onDemandEntryHandler({
       doneCallbacks!.emit(page)
     }
 
-    invalidator.doneBuilding()
+    invalidator.doneBuilding([...COMPILER_KEYS])
   })
 
   const pingIntervalTime = Math.max(1000, Math.min(5000, maxInactiveAge))

diff --git a/test/development/acceptance-app/app-hmr-changes.test.ts b/test/development/acceptance-app/app-hmr-changes.test.ts
@@ -0,0 +1,61 @@
+import { createNextDescribe, FileRef } from 'e2e-utils'
+import { check, hasRedbox, waitFor } from 'next-test-utils'
+import path from 'path'
+
+createNextDescribe(
+  'Error overlay - RSC build errors',
+  {
+    files: new FileRef(path.join(__dirname, 'fixtures', 'app-hmr-changes')),
+    dependencies: {
+      '@next/mdx': 'canary',
+      'react-wrap-balancer': '^0.2.4',
+      'next-tweet': '^0.6.0',
+      '@mdx-js/react': '^2.3.0',
+      tailwindcss: '^3.2.6',
+      typescript: 'latest',
+      '@types/react': '^18.0.28',
+      '@types/react-dom': '^18.0.10',
+      'image-size': '^1.0.2',
+      autoprefixer: '^10.4.13',
+    },
+  },
+  ({ next }) => {
+    it('should handle successive HMR changes with errors correctly', async () => {
+      const browser = await next.browser('/2020/develop-preview-test')
+
+      await check(
+        () => browser.eval('document.documentElement.innerHTML'),
+        /A few years ago I tweeted/
+      )
+
+      const pagePath = 'app/(post)/2020/develop-preview-test/page.mdx'
+      const originalPage = await next.readFile(pagePath)
+
+      const break1 = originalPage.replace('break 1', '<Figure>')
+
+      await next.patchFile(pagePath, break1)
+
+      const break2 = break1.replace('{/* break point 2 */}', '<Figure />')
+
+      await next.patchFile(pagePath, break2)
+
+      for (let i = 0; i < 5; i++) {
+        await next.patchFile(pagePath, break2.replace('break 3', '<Hello />'))
+
+        await next.patchFile(pagePath, break2)
+        expect(await hasRedbox(browser, true)).toBe(true)
+
+        await next.patchFile(pagePath, break1)
+        await waitFor(100)
+
+        await next.patchFile(pagePath, originalPage)
+        expect(await hasRedbox(browser, false)).toBe(false)
+      }
+
+      await check(
+        () => browser.eval('document.documentElement.innerHTML'),
+        /A few years ago I tweeted/
+      )
+    })
+  }
+)
diff --git a/...ance-app/fixtures/app-hmr-changes/app/(post)/2020/develop-preview-test/page.mdx b/...ance-app/fixtures/app-hmr-changes/app/(post)/2020/develop-preview-test/page.mdx
@@ -0,0 +1,213 @@
+A few years ago I tweeted a simple guideline that
+[challenged conventional wisdom](https://twitter.com/swyx/status/1261202288476971008)
+and became the norm for many people learning
+[how to test their frontends](https://kentcdodds.com/blog/write-tests)
+effectively.
+
+Writing tests is an **investment**, and like any other investment,
+it has to be evaluated in terms of its **return** and **risks** (e.g.:
+[opportunity costs](https://en.wikipedia.org/wiki/Opportunity_cost)) it incurs in.
+
+The "not too many" keys in on that. There is, after all, too much of a
+good thing when it comes to spending time writing tests instead of
+features. We can see an application of this insight also to availability
+by [Google's popular SRE book](https://landing.google.com/sre/sre-book/chapters/embracing-risk/):
+
+> You might expect Google to try to build 100% reliable services—ones that
+> never fail. It turns out that past a certain point, however, increasing
+> reliability is worse for a service (and its users) rather than better!
+> Extreme reliability comes at a cost: maximizing stability limits how fast
+> new features can be developed and how quickly products can be delivered to
+> users, and dramatically increases their cost, which in turn reduces the
+> numbers of features a team can afford to offer
+
+In this essay I want to make the case that
+**prioritizing end-to-end (E2E) testing** for the critical parts of
+your app will reduce risk and give you the best return. Further, I'll show
+how you can adopt this methodology in mere minutes.
+
+## Why end-to-end?
+
+In addition to integration tests, I want to now make the case that modern
+deployment workflows in combination with serverless testing solutions can
+now fully revert the conventional testing pyramid:
+
+<Image
+  alt={
+    <span>
+      Martin Fowler's conventional{' '}
+      <a
+        href="https://martinfowler.com/articles/practical-test-pyramid.html#TheTestPyramid"
+        target="_blank"
+      >
+        testing pyramid
+      </a>
+      . What if 🐢 and 💲 went away?
+    </span>
+  }
+  src="/images/develop-preview-test/pyramid.jpg"
+/>
+
+As it turns out,
+[focusing on the customer first](https://blog.aboutamazon.com/company-news/2016-letter-to-shareholders)
+is the best way to run a business, but also the best way to ascertain
+software quality.
+
+You might be using the fanciest new compiler, the newest type system, but
+won't do well if, after you ship, your site doesn't load in Chrome at all
+because you forgot to send the right HTTP headers for your `.js` files.
+Or you use features not compatible with Safari.
+
+Modern cloud software has a great deal of complexity that cannot be
+ascertained "in the lab", or by running artificial tests in your computer.
+We normally deploy software to dozens of operating systems, mobile
+devices, web browsers, networks of varying stability and performance… And
+the environment is always changing, as we take more and more dependencies
+on hosted services and third-party APIs.
+
+Furthermore, software quality goes beyond correctness. It's not just about
+doing "the right thing".
+[Quality encapsulates reliability](https://twitter.com/garybernhardt/status/1007699924866093056)
+(whether your program stays working correctly over time) and
+[performance](https://craigmod.com/essays/fast_software/)
+(it must do the right thing quickly).
+
+## End-to-end made possible
+
+First, back when I argued for focusing on integration in 2016 and made no
+mention of end-to-end tests, we didn't yet have the deployment
+infrastructure we have today.
+
+With [Vercel](https://vercel.com/),
+every push to git gets deployed and given an URL. This can be done cost
+effectively by focusing on incremental computation, where the underlying
+static files and serverless functions used to power a deployment are
+mostly re-used from deploy to deploy.
+
+<Image
+  alt={
+    <span>
+      An example of a PR automatically deployed{' '}
+      <a
+        target="_blank"
+        href="https://github.com/rauchg/blog/pull/35#issuecomment-570270061"
+      >
+        for this very blog
+      </a>
+    </span>
+  }
+  src="/images/develop-preview-test/github-comment.jpg"
+/>
+
+Just like GitHub is capable of keeping the history of your code forever,
+so can Vercel. Because deploys are
+[stateless](/2020/2019-in-review#static-is-the-new-dynamic),
+the resources are only used on-demand and scale infinitely with traffic.
+
+That gets rid of an obvious initial objection: that it would be difficult
+or costly to perform end-to-end tests against a live individualized
+version of your website. Instead of thinking about a single staging server
+that end-to-end tests are run against, now you can have millions of
+concurrent "staging" environments.
+
+I say "staging" in quotes because in reality, you run tests against the
+real production infrastructure, which has enormous reliability benefits.
+Not only are certain
+[features these days only enabled when SSL is on](https://developer.mozilla.org/en-US/docs/Web/Security/Secure_Contexts/features_restricted_to_secure_contexts),
+but having every other production feature enabled, like brotli
+compression and HTTP/2, reduces your risk of outages:
+
+<Tweet
+  id="921185541487341568"
+  caption="The risks of environment differences, as explained by Kubernetes' creator"
+/>
+
+Having made staging instant, prod-like and cost-effective, the remaining
+objection is that running the actual end-to-end tests would be expensive
+or slow, going by the pyramid above.
+
+However, the introduction of
+[puppeteer](https://github.com/puppeteer/puppeteer),
+Chrome's headless web browser, combined with serverless compute that can
+instantly scale to thousands of concurrent executions (see this
+[Stanford paper](https://stanford.edu/~sadjad/gg-paper.pdf)
+for an example) are <b>now making E2E both fast and cheap</b>.
+
+## End-to-end made easy
+
+To show the **Develop -> Preview -> Test** workflow in action, I'm
+going to demonstrate how easily I can add an E2E test for
+[my open-source blog](https://github.com/rauchg/blog)
+using [Vercel](https://vercel.com) in combination with
+[Checkly](https://www.checklyhq.com)
+on top of GitHub.
+
+First, I'm going to define an end-to-end test to ascertain that my blog
+always contains the string "Guillermo Rauch" in the browser title. As I'm
+writing this, it's not the case, so I expect this to fail.
+
+Checkly gives me full access to the Node and puppeteer APIs. Notice I
+don't have to write any ceremony, setup or scheduling logic. I'm "inside"
+the function that will later be invoked:
+
+<Image
+  alt="The ENVIRONMENT_URL env variable will be populated with each preview deploy URL"
+  src="/images/develop-preview-test/checkly.jpg"
+/>
+
+Then, I installed the
+[Checkly GitHub app](https://www.checklyhq.com/docs/cicd/github/#setting-up-your-github-integration)
+and under the CI/CD tab of the check, I linked it to my
+`rauchg/blog` repository.
+
+I created a git branch where I introduced the `<title>`
+tag for my blog, but on purpose
+[I misspelt my name](https://github.com/rauchg/blog/pull/53/commits/b5c30eaef41944f36cf14e7d7f8be9be9953709f).
+
+As soon as I created my pull request, the check was already failing. In
+just a few seconds, I pushed, deployed, a headless browser simulating a
+visitor ran, and my error was exposed:
+
+break 1
+
+<Image
+  alt="I can also configure my testing check as mandatory and make this PR unmergeable"
+  src="/images/develop-preview-test/failing-check.jpg"
+/>
+
+break 2
+
+break 3
+
+This happened with absolutely zero additional configuration. Vercel
+supplied a URL, Checkly tested it, GitHub was notified. After pushing
+again, the check re-runs automatically:
+
+<Image
+  alt="Each commit gets its own deploy preview, and its own checks"
+  src="/images/develop-preview-test/commits.jpg"
+/>
+
+With my typo fixed, I'm free to merge. Merging to master will deploy my
+changes to `rauchg.com` automatically.
+
+<Image
+  alt="After pressing the green button, we go live. With confidence."
+  src="/images/develop-preview-test/github-green.jpg"
+/>
+
+## Conclusion
+
+Notably, Checkly allows us to configure multiple locations in the world
+that the tests get executed from, as well as
+
+<b>invoking our checks over time</b>.
+
+Leslie Lamport
+[famously said](https://lamport.azurewebsites.net/pubs/distributed-system.txt)
+that a distributed system is one where "the failure of a computer you
+didn't even know existed can render your own computer unusable".
+
+As our systems become more complex, with ever-changing platforms and
+dependencies on other cloud systems, continuously testing just like our
+users do is our best bet to tame the chaos.
diff --git a/test/development/acceptance-app/fixtures/app-hmr-changes/app/(post)/components/a.tsx b/test/development/acceptance-app/fixtures/app-hmr-changes/app/(post)/components/a.tsx
@@ -0,0 +1,13 @@
+import Link from 'next/link'
+
+export function A({ children, className = '', href, ...props }) {
+  return (
+    <Link
+      href={href}
+      className={`border-b text-gray-600 border-gray-300 transition-[border-color] hover:border-gray-600 dark:text-white dark:border-gray-500 dark:hover:border-white ${className}`}
+      {...props}
+    >
+      {children}
+    </Link>
+  )
+}
diff --git a/.../development/acceptance-app/fixtures/app-hmr-changes/app/(post)/components/blockquote.tsx b/.../development/acceptance-app/fixtures/app-hmr-changes/app/(post)/components/blockquote.tsx
@@ -0,0 +1,9 @@
+import type { ReactNode } from 'react'
+
+export function Blockquote({ children }: { children: ReactNode }) {
+  return (
+    <blockquote className="my-5 text-gray-500 pl-3 border-l-4 dark:border-gray-600 dark:text-gray-400">
+      {children}
+    </blockquote>
+  )
+}
diff --git a/test/development/acceptance-app/fixtures/app-hmr-changes/app/(post)/components/callout.tsx b/test/development/acceptance-app/fixtures/app-hmr-changes/app/(post)/components/callout.tsx
@@ -0,0 +1,6 @@
+export const Callout = ({ emoji = null, text }) => (
+  <div className="bg-gray-200 dark:bg-[#333] dark:text-gray-300 flex items-start p-3 my-6 text-base">
+    <span className="block w-6 text-center text-xl mr-2">{emoji}</span>
+    <span className="block grow">{text}</span>
+  </div>
+)
diff --git a/test/development/acceptance-app/fixtures/app-hmr-changes/app/(post)/components/caption.tsx b/test/development/acceptance-app/fixtures/app-hmr-changes/app/(post)/components/caption.tsx
@@ -0,0 +1,12 @@
+import Balancer from 'react-wrap-balancer'
+import type { ReactNode } from 'react'
+
+export function Caption({ children }: { children: ReactNode }) {
+  return (
+    <p className="text-xs my-3 font-mono text-gray-500 text-center leading-normal">
+      <Balancer>
+        <span className="[&>a]:post-link">{children}</span>
+      </Balancer>
+    </p>
+  )
+}
diff --git a/test/development/acceptance-app/fixtures/app-hmr-changes/app/(post)/components/footnotes.tsx b/test/development/acceptance-app/fixtures/app-hmr-changes/app/(post)/components/footnotes.tsx
@@ -0,0 +1,28 @@
+import { A } from './a'
+import { P } from './p'
+
+export const FootNotes = ({ children }) => (
+  <div className="text-base before:w-[200px] before:m-auto before:content[''] before:border-t before:border-gray-300 dark:before:border-[#444] before:block before:my-10">
+    {children}
+  </div>
+)
+
+export const Ref = ({ id }) => (
+  <a
+    href={`#f${id}`}
+    id={`s${id}`}
+    className="relative text-xs top-[-5px] no-underline"
+  >
+    [{id}]
+  </a>
+)
+
+export const FootNote = ({ id, children }) => (
+  <P>
+    {id}.{' '}
+    <A href={`#s${id}`} id={`f${id}`} className="no-underline">
+      ^
+    </A>{' '}
+    {children}
+  </P>
+)