Skip to content

Commit

Permalink
Next.js image optimization fixes (#6143)
Browse files Browse the repository at this point in the history
* fix Next.js optimization for latest version

* fix unit tests

* `isUsingNextImageInAppDirectory` tests

* move Next version logic to getNextVersion + tests

* remove findDependency stubs

---------

Co-authored-by: Alex Astrum <[email protected]>
  • Loading branch information
leoortizz and alexastrum authored Jul 19, 2023
1 parent 1f5f2ac commit ad4e144
Show file tree
Hide file tree
Showing 5 changed files with 165 additions and 29 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,5 @@
- Fixed issue where Flutter Web is not detected as a web framework. (#6085)
- Add better messages for API permissions failures that direct the user to the URL to enable the API. (#6130)
- Fixes issue caused by adding type checks in #5906.
- Fixed `next/image` component in app directory for Next.js > 13.4.9. (#6143)
- Fixed bug where Next.js Image Optimization in the app directory was not requiring a Cloud Function. (#6143)
20 changes: 4 additions & 16 deletions src/frameworks/next/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,7 @@ import {
getNonStaticServerComponents,
getHeadersFromMetaFiles,
cleanI18n,
usesAppDirRouter,
usesNextImage,
hasUnoptimizedImage,
getNextVersion,
} from "./utils";
import { NODE_VERSION, NPM_COMMAND_TIMEOUT_MILLIES, SHARP_VERSION, I18N_ROOT } from "../constants";
import type {
Expand Down Expand Up @@ -80,10 +78,6 @@ export const docsUrl = "https://firebase.google.com/docs/hosting/frameworks/next

const DEFAULT_NUMBER_OF_REASONS_TO_LIST = 5;

function getNextVersion(cwd: string): string | undefined {
return findDependency("next", { cwd, depth: 0, omitDev: false })?.version;
}

function getReactVersion(cwd: string): string | undefined {
return findDependency("react-dom", { cwd, omitDev: false })?.version;
}
Expand Down Expand Up @@ -132,7 +126,7 @@ export async function build(dir: string): Promise<BuildResult> {
reasonsForBackend.add("middleware");
}

if (await isUsingImageOptimization(join(dir, distDir))) {
if (await isUsingImageOptimization(dir, distDir)) {
reasonsForBackend.add(`Image Optimization`);
}

Expand Down Expand Up @@ -544,14 +538,8 @@ export async function ɵcodegenFunctionsDirectory(sourceDir: string, destDir: st
await copy(join(sourceDir, "public"), join(destDir, "public"));
}

// Add the `sharp` library if `/app` folder exists (i.e. Next.js 13+)
// or usesNextImage in `export-marker.json` is set to true.
// As of (10/2021) the new Next.js 13 route is in beta, and usesNextImage is always being set to false
// if the image component is used in pages coming from the new `/app` routes.
if (
!(await hasUnoptimizedImage(sourceDir, distDir)) &&
(usesAppDirRouter(sourceDir) || (await usesNextImage(sourceDir, distDir)))
) {
// Add the `sharp` library if app is using image optimization
if (await isUsingImageOptimization(sourceDir, distDir)) {
packageJson.dependencies["sharp"] = SHARP_VERSION;
}

Expand Down
67 changes: 57 additions & 10 deletions src/frameworks/next/utils.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import { existsSync } from "fs";
import { pathExists } from "fs-extra";
import { basename, extname, join, posix } from "path";
import { readFile } from "fs/promises";
import { Glob } from "glob";
import type { PagesManifest } from "next/dist/build/webpack/plugins/pages-manifest-plugin";
import { coerce } from "semver";

import { isUrl, readJSON } from "../utils";
import { findDependency, isUrl, readJSON } from "../utils";
import type {
RoutesManifest,
ExportMarker,
Expand All @@ -26,7 +29,6 @@ import {
MIDDLEWARE_MANIFEST,
} from "./constants";
import { dirExistsSync, fileExistsSync } from "../../fsutils";
import { readFile } from "fs/promises";

export const I18N_SOURCE = /\/:nextInternalLocale(\([^\)]+\))?/;

Expand Down Expand Up @@ -159,6 +161,7 @@ export function usesAppDirRouter(sourceDir: string): boolean {
const appPathRoutesManifestPath = join(sourceDir, APP_PATH_ROUTES_MANIFEST);
return existsSync(appPathRoutesManifestPath);
}

/**
* Check if the project is using the next/image component based on the export-marker.json file.
* @param sourceDir location of the source directory
Expand Down Expand Up @@ -209,25 +212,55 @@ export async function isUsingMiddleware(dir: string, isDevMode: boolean): Promis
/**
* Whether image optimization is being used
*
* @param dir path to `distDir` - where the manifests are located
* @param projectDir path to the project directory
* @param distDir path to `distDir` - where the manifests are located
*/
export async function isUsingImageOptimization(dir: string): Promise<boolean> {
let { isNextImageImported } = await readJSON<ExportMarker>(join(dir, EXPORT_MARKER));
export async function isUsingImageOptimization(
projectDir: string,
distDir: string
): Promise<boolean> {
let isNextImageImported = await usesNextImage(projectDir, distDir);

// App directory doesn't use the export marker, look it up manually
if (!isNextImageImported && isUsingAppDirectory(dir)) {
isNextImageImported = (await readFile(join(dir, "server", "client-reference-manifest.js")))
.toString()
.includes("node_modules/next/dist/client/image.js");
if (!isNextImageImported && isUsingAppDirectory(join(projectDir, distDir))) {
if (await isUsingNextImageInAppDirectory(projectDir, distDir)) {
isNextImageImported = true;
}
}

if (isNextImageImported) {
const imagesManifest = await readJSON<ImagesManifest>(join(dir, IMAGES_MANIFEST));
const imagesManifest = await readJSON<ImagesManifest>(
join(projectDir, distDir, IMAGES_MANIFEST)
);
return !imagesManifest.images.unoptimized;
}

return false;
}

/**
* Whether next/image is being used in the app directory
*/
export async function isUsingNextImageInAppDirectory(
projectDir: string,
nextDir: string
): Promise<boolean> {
const { found: files } = new Glob(
join(projectDir, nextDir, "server", "**", "*client-reference-manifest.js")
);

for await (const filepath of files) {
const fileContents = await readFile(filepath);

// Return true when the first file containing the next/image component is found
if (fileContents.includes("node_modules/next/dist/client/image")) {
return true;
}
}

return false;
}

/**
* Whether Next.js app directory is being used
*
Expand Down Expand Up @@ -360,3 +393,17 @@ export async function getBuildId(distDir: string): Promise<string> {

return buildId.toString();
}

/**
* Get Next.js version in the following format: `major.minor.patch`, ignoring
* canary versions as it causes issues with semver comparisons.
*/
export function getNextVersion(cwd: string): string | undefined {
const dependency = findDependency("next", { cwd, depth: 0, omitDev: false });
if (!dependency) return undefined;

const nextVersionSemver = coerce(dependency.version);
if (!nextVersionSemver) return dependency.version;

return nextVersionSemver.toString();
}
Loading

0 comments on commit ad4e144

Please sign in to comment.