Make fs-ext dependency optional (#2322)

## Motivation for the change, related issues

Some folks are encountering issue with building the fs-ext module
containing a native Node.js add-on.

Fixes #2308

## Implementation details

This PR makes the fs-ext dependency optional. This means that the
dependency will be proactively installed by `npm`, but if an optional
dependency fails to install, it does not fail the entire npm install.

## Testing Instructions (or ideally a Blueprint)

- Run `npm ci`
- Confirm that `fs-ext` is installed under node_modules
- Edit packages/php-wasm/node/src/lib/file-lock-manager-for-node.ts and
add a console.log() statement to `flockSyncNoOp()`
- Make local dir test dir
- Run `npx nx unbuilt-jspi playground-cli server
--mountBeforeInstall=<local-test-dir-path>:/wordpress`
- Confirm it starts with no errors and no `flockSyncNoOp` messages.
- Delete `node_modules/fs-ext`
- Run `npx nx unbuilt-jspi playground-cli server
--mountBeforeInstall=<local-test-dir-path>:/wordpress`
- Confirm it starts successfully and prints `flockSyncNoP` console
messages.

Author: brandonpayton

package-lock.json

@@ -77,7 +77,6 @@
 		"diff3": "0.0.4",
 		"express": "4.21.2",
 		"file-saver": "^2.0.5",
-		"fs-ext": "2.1.1",
 		"fs-extra": "11.1.1",
 		"ini": "4.1.2",
 		"octokit": "3.1.2",
@@ -190,5 +189,8 @@
 	"engines": {
 		"node": ">=20.18.3",
 		"npm": ">=10.1.0"
+	},
+	"optionalDependencies": {
+		"fs-ext": "2.1.1"
 	}
 }

package.json

@@ -73,54 +73,6 @@ const dirnamePlugin = {
 	},
 };
 
-// Create a require function that can be used in the nativeNodeModulesPlugin.
-const require = createRequire(import.meta.url);
-
-/**
- * This is a plugin for handling native Node.js modules.
- *
- * Taken from:
- * https://github.com/evanw/esbuild/issues/1051#issuecomment-806325487
- */
-const nativeNodeModulesPlugin = {
-	name: 'native-node-modules',
-	setup(build) {
-		// If a ".node" file is imported within a module in the "file" namespace, resolve
-		// it to an absolute path and put it into the "node-file" virtual namespace.
-		build.onResolve({ filter: /\.node$/, namespace: 'file' }, (args) => ({
-			path: require.resolve(args.path, { paths: [args.resolveDir] }),
-			namespace: 'node-file',
-		}));
-
-		// Files in the "node-file" virtual namespace call "require()" on the
-		// path from esbuild of the ".node" file in the output directory.
-		build.onLoad({ filter: /.*/, namespace: 'node-file' }, (args) => ({
-			contents: `
-				import path from ${JSON.stringify(args.path)}
-				try { module.exports = require(path) }
-				catch {}
-			`,
-		}));
-
-		// If a ".node" file is imported within a module in the "node-file" namespace, put
-		// it in the "file" namespace where esbuild's default loading behavior will handle
-		// it. It is already an absolute path since we resolved it to one above.
-		build.onResolve(
-			{ filter: /\.node$/, namespace: 'node-file' },
-			(args) => ({
-				path: args.path,
-				namespace: 'file',
-			})
-		);
-
-		// Tell esbuild's default loading behavior to use the "file" loader for
-		// these ".node" files.
-		let opts = build.initialOptions;
-		opts.loader = opts.loader || {};
-		opts.loader['.node'] = 'file';
-	},
-};
-
 /**
  * This is a plugin for handling imports ending with ?url.
  */
@@ -171,13 +123,13 @@ async function build() {
 		format: 'cjs',
 		bundle: true,
 		tsconfig: 'packages/php-wasm/node/tsconfig.json',
-		external: ['@php-wasm/*', '@wp-playground/*', 'ws'],
+		external: ['@php-wasm/*', '@wp-playground/*', 'ws', 'fs-ext'],
 		loader: {
 			'.php': 'text',
 			'.ini': 'file',
 			'.wasm': 'file',
 		},
-		plugins: [dirnamePlugin, nativeNodeModulesPlugin, importUrlPlugin],
+		plugins: [dirnamePlugin, importUrlPlugin],
 	});
 
 	await esbuild.build({
@@ -203,7 +155,14 @@ const __dirname = import.meta.dirname;
 		packages: 'external',
 		bundle: true,
 		tsconfig: 'packages/php-wasm/node/tsconfig.json',
-		external: ['@php-wasm/*', '@wp-playground/*', 'ws', 'fs', 'path'],
+		external: [
+			'@php-wasm/*',
+			'@wp-playground/*',
+			'ws',
+			'fs',
+			'path',
+			'fs-ext',
+		],
 		supported: {
 			'dynamic-import': true,
 			'top-level-await': true,
@@ -214,7 +173,7 @@ const __dirname = import.meta.dirname;
 			'.ini': 'file',
 			'.wasm': 'file',
 		},
-		plugins: [dirnamePlugin, nativeNodeModulesPlugin, importUrlPlugin],
+		plugins: [dirnamePlugin, importUrlPlugin],
 	});
 
 	fs.copyFileSync(

packages/php-wasm/node/build.js

@@ -1,6 +1,10 @@
 import { logger } from '@php-wasm/logger';
 import { openSync, closeSync } from 'fs';
-import { flockSync as nativeFlockSync } from 'fs-ext';
+
+type NativeFlockSync = (
+	fd: number,
+	flags: 'sh' | 'ex' | 'shnb' | 'exnb' | 'un'
+) => void;
 
 import type {
 	FileLockManager,
@@ -16,6 +20,7 @@ type LockMode = 'exclusive' | 'shared' | 'unlock';
 type NativeLock = {
 	fd: number;
 	mode: LockMode;
+	nativeFlockSync: NativeFlockSync;
 };
 
 type LockedRange = RequestedRangeLock & {
@@ -31,9 +36,20 @@ const MAX_64BIT_OFFSET = BigInt(2n ** 64n - 1n);
  * It provides methods for locking and unlocking files, as well as finding conflicting locks.
  */
 export class FileLockManagerForNode implements FileLockManager {
+	nativeFlockSync: NativeFlockSync;
 	locks: Map<string, FileLock>;
 
-	constructor() {
+	/**
+	 * Create a new FileLockManagerForNode instance.
+	 *
+	 * @param nativeFlockSync A synchronous flock() function to lock files via the host OS.
+	 */
+	constructor(
+		nativeFlockSync: NativeFlockSync = function flockSyncNoOp() {
+			/* do nothing */
+		}
+	) {
+		this.nativeFlockSync = nativeFlockSync;
 		this.locks = new Map();
 	}
 
@@ -51,7 +67,11 @@ export class FileLockManagerForNode implements FileLockManager {
 				return true;
 			}
 
-			const maybeLock = FileLock.maybeCreate(path, op.type);
+			const maybeLock = FileLock.maybeCreate(
+				path,
+				op.type,
+				this.nativeFlockSync
+			);
 			if (maybeLock === undefined) {
 				return false;
 			}
@@ -82,7 +102,11 @@ export class FileLockManagerForNode implements FileLockManager {
 				return true;
 			}
 
-			const maybeLock = FileLock.maybeCreate(path, requestedLock.type);
+			const maybeLock = FileLock.maybeCreate(
+				path,
+				requestedLock.type,
+				this.nativeFlockSync
+			);
 			if (maybeLock === undefined) {
 				return false;
 			}
@@ -177,7 +201,8 @@ export class FileLock {
 	 */
 	static maybeCreate(
 		path: string,
-		mode: Exclude<WholeFileLock['type'], 'unlocked'>
+		mode: Exclude<WholeFileLock['type'], 'unlocked'>,
+		nativeFlockSync: NativeFlockSync
 	): FileLock | undefined {
 		let fd;
 		try {
@@ -186,7 +211,7 @@ export class FileLock {
 			const flockFlags = mode === 'exclusive' ? 'exnb' : 'shnb';
 			nativeFlockSync(fd, flockFlags);
 
-			const nativeLock: NativeLock = { fd, mode };
+			const nativeLock: NativeLock = { fd, mode, nativeFlockSync };
 			return new FileLock(nativeLock);
 		} catch {
 			if (fd !== undefined) {
@@ -586,7 +611,7 @@ export class FileLock {
 			'un';
 
 		try {
-			nativeFlockSync(this.nativeLock.fd, flockFlags);
+			this.nativeLock.nativeFlockSync(this.nativeLock.fd, flockFlags);
 			this.nativeLock.mode = requiredNativeLockType;
 			return true;
 		} catch {

packages/php-wasm/node/src/lib/file-lock-manager-for-node.ts

@@ -475,7 +475,17 @@ export async function runCLI(args: RunCLIArgs): Promise<RunCLIServer> {
 
 	// Declare file lock manager outside scope of startServer
 	// so we can look at it when debugging request handling.
-	const fileLockManager = new FileLockManagerForNode();
+	const nativeFlockSync = await import('fs-ext')
+		.then((m) => m.flockSync)
+		.catch(() => {
+			logger.warn(
+				'The fs-ext package is not installed. ' +
+					'Internal file locking will not be integrated with ' +
+					'host OS file locking.'
+			);
+			return undefined;
+		});
+	const fileLockManager = new FileLockManagerForNode(nativeFlockSync);
 
 	let wordPressReady = false;
 

packages/playground/cli/src/run-cli.ts

@@ -82,6 +82,7 @@ export default defineConfig({
 				'readline',
 				'worker_threads',
 				'url',
+				'fs-ext',
 			],
 		},
 		lib: {