Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a workaround for the not-found app directory issue #418

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions .changeset/little-hotels-wave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
'eslint-plugin-next-on-pages': patch
'@cloudflare/next-on-pages': patch
---

Add workaround so that people can use standard not-found routes in their app directory applications

The problem is that:

- if there's a static not-found route in app dir, that generates a serverless (edge incompatible) function (\_not-found)
- if there's a dynamic not-found route in app dir, that generates two serverless (edge incompatible) functions (\_not-found, \_error)

The workaround being introduced here is:

- if there's a static not-found route in app dir, we delete the generated \_not-found serverless function
(which is not needed as we can just fallback to the static 404 html)
- if there's a dynamic not-found route in app dir, we can't actually fix it but provide a warning for the user

Besides the above the `no-app-not-found-runtime` eslint rule has been introduced to try to help developers avoid
the issue
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# `next-on-pages/no-app-not-found-runtime`

[Not found](https://nextjs.org/docs/app/api-reference/file-conventions/not-found) app router components are not currently supported with `@cloudflare/next-on-pages`, the only current alternative is not to export a runtime and making sure that the component doesn't have runtime logic. In such a way a static 404 page gets generated during the build time and served to users.

> **Note**
> This restriction applies only to top-level not-found pages (present in the `app` directory), not-found pages
> nested inside routes are handled correctly and can contain runtime logic.

## ❌ Invalid Code

```js
// app/not-found.jsx

export const runtime = 'edge';
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```

> **Warning**
> The following code is invalid but not caught by the rule, please be aware of such limitation

```js
// app/not-found.jsx

import { cookies } from 'next/headers';

export default function NotFound() {
// notice the runtime cookie retrieval
const cookieStore = cookies();
const theme = cookieStore.get('theme');

return (
<div className={`not-found--${theme}`}>
<h2>Not Found</h2>
</div>
);
}
```

## ✅ Valid Code

```js
// app/not-found.jsx

export default function NotFound() {
return (
<div>
<h2>Not Found</h2>
</div>
);
}
```
3 changes: 3 additions & 0 deletions packages/eslint-plugin-next-on-pages/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,20 @@
import noNodeJsRuntime from './rules/no-nodejs-runtime';
import noUnsupportedConfigs from './rules/no-unsupported-configs';
import noAppNotFound from './rules/no-app-not-found-runtime';

import type { ESLint } from 'eslint';

const config: ESLint.Plugin = {
rules: {
'no-nodejs-runtime': noNodeJsRuntime,
'no-unsupported-configs': noUnsupportedConfigs,
'no-app-not-found': noAppNotFound,
},
configs: {
recommended: {
plugins: ['eslint-plugin-next-on-pages'],
rules: {
'next-on-pages/no-app-not-found': 'error',
'next-on-pages/no-nodejs-runtime': 'error',
'next-on-pages/no-unsupported-configs': 'error',
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import type { Rule } from 'eslint';

const rule: Rule.RuleModule = {
create: context => {
const isAppNotFoundRoute = new RegExp(
`${context.cwd}/app/not-found\\.(tsx|jsx)`,
).test(context.filename);
return {
ExportNamedDeclaration: node => {
if (!isAppNotFoundRoute) {
// This rule only applies to app/not-found routes
return;
}

const declaration = node.declaration;
if (
declaration?.type === 'VariableDeclaration' &&
declaration.declarations.length === 1 &&
declaration.declarations[0]?.id.type === 'Identifier' &&
declaration.declarations[0].id.name === 'runtime' &&
declaration.declarations[0].init?.type === 'Literal'
) {
context.report({
message:
'Only static not-found pages are currently supported, please remove the runtime export.' +
context.filename,
node,
fix: fixer => fixer.remove(node),
});
}
},
};
},
meta: {
fixable: 'code',
docs: {
url: 'https://github.com/cloudflare/next-on-pages/blob/main/packages/eslint-plugin-next-on-pages/docs/rules/no-app-not-found-runtime.md',
},
},
};

export = rule;
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { processEdgeFunctions } from './edgeFunctions';
import { processPrerenderFunctions } from './prerenderFunctions';
import type { CollectedFunctionIdentifiers } from './dedupeEdgeFunctions';
import { dedupeEdgeFunctions } from './dedupeEdgeFunctions';
import { checkForInvalidFunctions } from './invalidFunctions';
import { checkInvalidFunctions } from './invalidFunctions';

/**
* Processes and dedupes the Vercel build output functions directory.
Expand All @@ -23,7 +23,7 @@ export async function processVercelFunctions(

await processEdgeFunctions(collectedFunctions);

await checkForInvalidFunctions(collectedFunctions);
await checkInvalidFunctions(collectedFunctions);

const identifiers = await dedupeEdgeFunctions(collectedFunctions, opts);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,25 @@
import { gtr as versionGreaterThan, coerce } from 'semver';
import { cliError } from '../../cli';
import { cliError, cliWarn } from '../../cli';
import { getPackageVersion } from '../packageManagerUtils';
import { stripFuncExtension } from '../../utils';
import type { CollectedFunctions, FunctionInfo } from './configs';
import { join, resolve } from 'path';

/**
* Checks if there are any invalid functions from the Vercel build output.
*
* If there are any invalid functions, an error message will be printed and the process will exit.
* If there are any invalid functions it will try to see if they are amendable and if the
* build output can still be used.
*
* If however the build output can't be used, an error message will be printed and the process will exit.
*
* @param collectedFunctions Collected functions from the Vercel build output.
*/
export async function checkForInvalidFunctions(
export async function checkInvalidFunctions(
collectedFunctions: CollectedFunctions,
): Promise<void> {
await tryToFixNotFoundRoute(collectedFunctions);

if (collectedFunctions.invalidFunctions.size > 0) {
await printInvalidFunctionsErrorMessage(
collectedFunctions.invalidFunctions,
Expand All @@ -22,6 +28,49 @@ export async function checkForInvalidFunctions(
}
}

/**
* Tries to fix potential not-found invalid functions from the Vercel build output.
*
* Static app/not-found.(jsx|tsx) pages generate an _not-found.func serverless function,
* that can be removed as we can fallback to the statically generated 404 page
*
* If the app/not-found.(jsx|tsx) contains runtime logic alongside the _not-found.func serverless
* function also an _error.func will be generated, in such a case we can only warn the user about
* it.
* (
* That's the only option because:
* - removing the _not-found.func and _error.func doesn't result in a working application
* - we don't have a guarantee that the _error.func hasn't been generated by something else
* and that the _not-found.func is that of a static app/not-found route
* )
*
* @param collectedFunctions Collected functions from the Vercel build output.
*/
async function tryToFixNotFoundRoute(
collectedFunctions: CollectedFunctions,
): Promise<void> {
const functionsDir = resolve('.vercel', 'output', 'functions');
const notFoundDir = join(functionsDir, '_not-found.func');
const errorDir = join(functionsDir, '_error.func');

const invalidNotFound = collectedFunctions.invalidFunctions.get(notFoundDir);
const invalidError = collectedFunctions.invalidFunctions.get(errorDir);

if (invalidNotFound && !invalidError) {
collectedFunctions.invalidFunctions.delete(notFoundDir);
const notFoundRscDir = join(functionsDir, '_not-found.rsc.func');
collectedFunctions.invalidFunctions.delete(notFoundRscDir);
}

if (invalidNotFound && invalidError) {
cliWarn(`
Warning: your app/not-found route might contain runtime logic, this is currently
not supported by @cloudflare/next-on-pages, if that's actually the case please
remove the runtime logic from your not-found route
`);
}
}

/**
* Prints an error message for the invalid functions from the Vercel build output.
*
Expand Down