Careful Using any in TypeScript - Here’s What Can Happen

The other day while browsing Twitter, I saw that Josh Goldberg (@JoshuaKGoldberg) tweeted out this:

And later said the root cause was “sketchy TypeScript types.”

I stopped scrolling.

You hear TypeScript enthusiasts praise the value of TypeScript and warn you never to use any. You believe it, but it’s helpful when they can point to a specific example. This was it! This was a real example in a popular open-source project.

Ryan Florence (@ryanflorence) once said the best way to level up is by reading source code.

Using Josh’s PR, we have the opportunity to read a real type error in production, study it, and learn from it! Brilliant!

In this article, we’re going to dive into a real scenario where using any led to production errors in Next.js.

Digging into the issue

The issue is an ambiguous TypeError that pops up during Next.js development if/when a network request to /_next/static/development/_devPagesManifest.json fails.

Sounds like an edge case, right? Well, you’d think so, but the original issue first showed up in September 2020 and received 41 👍🏼. From a maintainer’s perspective, that’s a significant number of affected users!

How do you reproduce it?

Josh gives us a list of steps to reproduce the issue:

1. Start any Next.js app in development mode
2. Add a <Link> somewhere on the page
3. Visit localhost:3000 (or equivalent) in Chrome (or equivalent)
4. In the network tab, right-click on the _devPagesManifest.json request and select Block request URL
5. Refresh the page

The key part is blocking the request in the network tab. He also provided a Stackblitz if you want an instant way to see this. And when you do this, you see the error on the right:

TypeError: Cannot read properties of undefined (reading 'includes')

What’s the root cause?

There are many reasons why the _devPagesManifest.json might fail to load including:

• flaky machine network layer (i.e. CI images)
• URL blocking (nginx configs)
• browser network blocking

Next.js doesn’t have control over those scenarios, but we can fix the sketchy type errors and improve the logging. First up, narrowing an any type annotation to the correct type:

- let pages: any, rewrites: any
+ let pages: string[], rewrites: any

This is inside packages/next/shared/lib/router/router.ts and is loosely typed as any instead of string[], which would have caught this page crash. Why? Well, let’s follow the path.

• this change in the code block above is inside router.ts and corresponds to the return value of getPageList
• by changing to string[], we now must return that type from getPageList
• inside page-loader.ts, we have a scenario where we override the types and assert that a value is truthy when it may undefined
• to be extra clear, the actual return type is not string[] at the moment; it’s actually string[] | undefined
• that means it isn’t guaranteed to always return string[], which is a problem

Let’s look at the fix for this next.

The fix is in a file called packages/next/client/page-loader.ts.

If you look near the bottom of this diff code block, you’ll see the use of the non-null assertion operator, !, which told TypeScript this value was not null or undefined.

We know this is sketchy because the original author left a comment saying, “this could be undefined. “

getPageList() {
    if (process.env.NODE_ENV === 'production') {
      return getClientBuildManifest().then((manifest) => manifest.sortedPages)
    } else {
      if (window.__DEV_PAGES_MANIFEST) {
        return window.__DEV_PAGES_MANIFEST.pages
      } else {
-       if (!this.promisedDevPagesManifest) {
-       // TODO: Decide what should happen when fetching fails instead of asserting
-       // @ts-ignore
+       this.promisedDevPagesManifest ||= fetch(
          `${this.assetPrefix}/_next/static/development/_devPagesManifest.json`
        )
          .then((res) => res.json())
          .then((manifest: { pages: string[] }) => {
            window.__DEV_PAGES_MANIFEST = manifest
            return manifest.pages
          })
          .catch((err) => {
            console.log(`Failed to fetch devPagesManifest:`, err)
+           throw new Error(
+             `Failed to fetch _devPagesManifest.json. Is something blocking that network request?`
+           )
          })
-        // TODO Remove this assertion as this could be undefined
-        return this.promisedDevPagesManifest!
+        return this.promisedDevPagesManifest
      }
    }
  }

He makes a small syntax cleanup using the Logical OR Operator, ||=, and cleans up the logic so that the fetch call either returns a value or throws an Error. This way, this.promisedDevPagesManifest is never undefined.

Lastly, if the fetch call fails, he throws an Error with a friendly error message.

Summary

As we saw in this situation, the combination of any and the ! lead to unexpected behavior and an unhelpful error message. Luckily, Josh found a reliable way to reproduce it and took it upon himself to submit a pull request to make this better for users.

Open source for the win!

If you can’t avoid sketchy type errors for whatever reason, do this:

1. Create type FixMeLater = any and use that instead of any
2. Open an issue for your TODO comment
3. Add a comment in the code with a brief explanation and link to the issue

This way, it’s more clear to future developers that you realize you’re taking a shortcut. And with the issue, it’s easier to keep track of your tech debt.

Thanks for reading! Happy TypeScript’ing!