Skip to content

Commit

Permalink
fix(middleware): 'instanceof Function' is dynamic code false-positive (
Browse files Browse the repository at this point in the history
…vercel#41249)

## 🐛 What's in there?

`foo instanceof Function` is wrongly considered as Dynamic code evaluation by our static analyzer.
This PR fixes it.

- [ ] Related issues linked using `fixes #number`
- [x] Integration tests added
- [x] Errors have a helpful link attached, see `contributing.md`

## 🧪 How to reproduce?

1. Create a simple repro in `examples` folder:
   ```js
   // examples/instance-of-function/pages/index.js
   export default function Home() {
     return <h1>home</h1>
   }
   
   // examples/instance-of-function/middleware.js
   import { NextResponse } from 'next/server'
   
   export default async function handler() {
     console.log('is arrow a function?', (() => {}) instanceof Function)
     return NextResponse.next()
   }
   ```
1. build with next `pnpm next build examples/instance-of-function`
   > the build fails
1. rebuild next to include PR's fix `pnpm build`
1. build with new next `pnpm next build examples/instance-of-function`
   > the build works

## 📔 Notes to reviewers

`hooks.expression.for(`${prefix}Function`).tap(NAME, handleExpression)` is actually legacy code from the original implementation. It's used when finding `Function` regardless of how it is used. We only want to find `new Function()` or `Function()`, which `hooks.calls` and `hooks.new` are covering.

`eval instanceof Function` is perfectly legit code on the edge, despite its uselessness :lol-think: 

Because we got multiple people asking "how do I relax this error when my code contains unreachable dynamic code evaluation", I've copy-pasted details about `config.unstable_allowDynamic` into the error page. Because users do not always click links :blob_shrug:
  • Loading branch information
feugy authored and Kikobeats committed Oct 24, 2022
1 parent e95693c commit b7fa48f
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 3 deletions.
14 changes: 13 additions & 1 deletion errors/edge-dynamic-code-evaluation.md
Expand Up @@ -29,6 +29,18 @@ export default async function middleware() {
```

In rare cases, your code could contain (or import) some dynamic code evaluation statements which _can not be reached at runtime_ and which can not be removed by treeshaking.
You can relax the check to allow specific files with your Middleware or Edge API Route exported [configuration](https://nextjs.org/docs/api-reference/edge-runtime#unsupported-apis).
You can relax the check to allow specific files with your Middleware or Edge API Route exported [configuration](https://nextjs.org/docs/api-reference/edge-runtime#unsupported-apis):

```typescript
export const config = {
runtime: 'experimental-edge', // for Edge API Routes only
unstable_allowDynamic: [
'/lib/utilities.js', // allows a single file
'/node_modules/function-bind/**', // use a glob to allow anything in the function-bind 3rd party module
],
}
```

`unstable_allowDynamic` is a glob, or an array of globs, ignoring dynamic code evaluation for specific files. The globs are relative to your application root folder.

Be warned that if these statements are executed on the Edge, _they will throw and cause a runtime error_.
2 changes: 0 additions & 2 deletions packages/next/build/webpack/plugins/middleware-plugin.ts
Expand Up @@ -553,8 +553,6 @@ Learn More: https://nextjs.org/docs/messages/node-module-in-edge-runtime`,
hooks.call.for(`${prefix}eval`).tap(NAME, handleWrapExpression)
hooks.call.for(`${prefix}Function`).tap(NAME, handleWrapExpression)
hooks.new.for(`${prefix}Function`).tap(NAME, handleWrapExpression)
hooks.expression.for(`${prefix}eval`).tap(NAME, handleExpression)
hooks.expression.for(`${prefix}Function`).tap(NAME, handleExpression)
hooks.call
.for(`${prefix}WebAssembly.compile`)
.tap(NAME, handleWrapWasmCompileExpression)
Expand Down
Expand Up @@ -387,4 +387,61 @@ describe('Edge runtime configurable guards', () => {
expect(output.stderr).toContain(TELEMETRY_EVENT_NAME)
})
})

describe.each([
{
title: 'Edge API',
url: routeUrl,
init() {
context.api.write(`
export default async function handler(request) {
return Response.json({ result: (() => {}) instanceof Function })
}
export const config = { runtime: 'experimental-edge' }
`)
},
},
{
title: 'Middleware',
url: middlewareUrl,
init() {
context.middleware.write(`
import { NextResponse } from 'next/server'
import { returnTrue } from './lib'
export default async function () {
(() => {}) instanceof Function
return NextResponse.next()
}
`)
},
},
])('$title with use of Function as a type', ({ init, url }) => {
beforeEach(() => init())

it('does not warn in dev at runtime', async () => {
context.app = await launchApp(context.appDir, context.appPort, appOption)
const res = await fetchViaHTTP(context.appPort, url)
await waitFor(500)
expect(res.status).toBe(200)
expect(context.logs.output).not.toContain(
`Dynamic Code Evaluation (e. g. 'eval', 'new Function') not allowed in Edge Runtime`
)
})

// eslint-disable-next-line jest/no-identical-title
it('build and does not warn at runtime', async () => {
const output = await nextBuild(context.appDir, undefined, {
stdout: true,
stderr: true,
})
expect(output.stderr).not.toContain(`Build failed`)
context.app = await nextStart(context.appDir, context.appPort, appOption)
const res = await fetchViaHTTP(context.appPort, url)
expect(res.status).toBe(200)
expect(context.logs.output).not.toContain(`warn`)
expect(context.logs.output).not.toContain(
`Dynamic Code Evaluation (e. g. 'eval', 'new Function') not allowed in Edge Runtime`
)
})
})
})

0 comments on commit b7fa48f

Please sign in to comment.