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.

Sign up for GitHub

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

Jump to bottom

Support installing WP as needed in Playground remote #1841

Open
wants to merge 7 commits into
base: trunk
Choose a base branch
from
Open

Support installing WP as needed in Playground remote #1841

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9ebeb51
Support installing WP as needed in Playground remote
brandonpayton Oct 4, 2024
2f53431
Fix vite dev build for website importing from remote package
brandonpayton Oct 8, 2024
ccde6fe
Export reusable looks-like-playground-dir function
brandonpayton Oct 8, 2024
5d555f9
Use universal looks-like-playground function
brandonpayton Oct 8, 2024
51dd315
Fix type error
brandonpayton Oct 8, 2024
3b1cffd
Tweak location of shared function
brandonpayton Oct 8, 2024
3b56699
Fix whitespace typo
brandonpayton Oct 8, 2024
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
5 changes: 4 additions & 1 deletion packages/playground/client/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,11 @@ export interface StartPlaygroundOptions {
*/
onBeforeBlueprint?: () => Promise<void>;
mounts?: Array<MountDescriptor>;
shouldInstallWordPress?: boolean;
/**
* Whether to install WordPress. Value may be boolean or 'auto'.
* If 'auto', WordPress will be installed if it is not already installed.
*/
shouldInstallWordPress?: boolean | 'auto'; /**
* The string prefix used in the site URL served by the currently
* running remote.html. E.g. for a prefix like `/scope:playground/`,
* the scope would be `playground`. See the `@php-wasm/scopes` package
Expand Down
1 change: 1 addition & 0 deletions packages/playground/remote/src/lib/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export * from './boot-playground-remote';
export * from './playground-client';
export { looksLikePlaygroundDirectory } from './worker-utils';
export {
MinifiedWordPressVersions,
MinifiedWordPressVersionsList,
Expand Down
21 changes: 20 additions & 1 deletion packages/playground/remote/src/lib/worker-thread.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
spawnHandlerFactory,
backfillStaticFilesRemovedFromMinifiedBuild,
hasCachedStaticFilesRemovedFromMinifiedBuild,
looksLikePlaygroundDirectory,
} from './worker-utils';
import { EmscriptenDownloadMonitor } from '@php-wasm/progress';
import { createMemoizedFetch } from './create-memoized-fetch';
Expand Down Expand Up @@ -72,7 +73,7 @@ export type WorkerBootOptions = {
scope: string;
withNetworking: boolean;
mounts?: Array<MountDescriptor>;
shouldInstallWordPress?: boolean;
shouldInstallWordPress?: boolean | 'auto';
};

/** @inheritDoc PHPClient */
Expand Down Expand Up @@ -188,6 +189,24 @@ export class PlaygroundWorkerEndpoint extends PHPWorker {
}

try {
if (shouldInstallWordPress === 'auto') {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How could we make this reusable for Playground CLI?

Copy link
Member Author

@brandonpayton brandonpayton Oct 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How could we make this reusable for Playground CLI?

Both Playground remote and CLI leverage bootWordPress(), but AFAICT, everything before that is custom setup for their individual context. I'll consider whether there is more we can share, but at the very least, we could implement the same logic for CLI.

The CLI's skipWordPressSetup argument is a potential conflict and maybe could be deprecated in favor of a shouldInstallWordPress argument. Or they could be complementary.

Thanks for drawing my attention to this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How could we make this reusable for Playground CLI?

Both Playground remote and CLI leverage bootWordPress(), but AFAICT, everything before that is custom setup for their individual context. I'll consider whether there is more we can share, but at the very least, we could implement the same logic for CLI.

I was thinking of saying the following:

To be able to implement this check in a way that works for both PHP and CLI, maybe such a feature should be moved into bootWordPress(). That way, we can write the check in PHP and...

But if we want to allow parallel downloads of PHP and WP resources in a web context, we need to check before PHP has been downloaded and loaded.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a reusable looksLikePlaygroundDirectory() function under @wp-playground/wordpress. The relative nature of the fileExists() predicate feels a little clunky, but I think this is workable for sharing the looks-like-playground-dir heuristic.

/**
  * Check if the given directory handle directory is a Playground directory.
  *
  * @param fileExists Function A function that checks if a file exists relative to an assumed directory.
  * @returns Promise<boolean> Whether the directory looks like a Playground directory.
  */
 export async function looksLikePlaygroundDirectory(
 	fileExists: (relativePath: string) => Promise<boolean>
 ) {
 	const results = await Promise.all(
 		['wp-config.php', 'wp-content/database/.ht.sqlite'].map(fileExists)
 	);
 	return results.every(Boolean);
 }

Copy link
Collaborator

@adamziel adamziel Oct 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's something similar in wp-now, you may want to review their "WordPress mode". One assumption this implementation makes is that we're running on SQLite whereas Playground CLI supports MySQL, too

// Default to installing WordPress unless we detect
// it in one of the mounts.
shouldInstallWordPress = true;

// NOTE: This check is insufficient if a complete WordPress
// installation is composed of multiple mounts.
for (const mount of mounts) {
const dirHandle = await directoryHandleFromMountDevice(
mount.device
);
if (await looksLikePlaygroundDirectory(dirHandle)) {
shouldInstallWordPress = false;
break;
}
}
}

// Start downloading WordPress if needed
let wordPressRequest = null;
if (shouldInstallWordPress) {
Expand Down
44 changes: 44 additions & 0 deletions packages/playground/remote/src/lib/worker-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,3 +245,47 @@ export async function getWordPressStaticZipUrl(php: PHP) {
}
return joinPaths('/', staticAssetsDirectory, 'wordpress-static.zip');
}

/**
* Check if the given directory handle directory is a Playground directory.
*
* @TODO: Create a shared package like @wp-playground/wordpress for such utilities
* and bring in the context detection logic from wp-now – only express it in terms of
* either abstract FS operations or isomorphic PHP FS operations.
* (we can't just use Node.js require('fs') in the browser, for example)
*
* @TODO: Reuse the "isWordPressInstalled" logic implemented in the boot protocol.
* Perhaps mount OPFS first, and only then check for the presence of the
* WordPress installation? Or, if not, perhaps implement a shared file access
* abstraction that can be used both with the PHP module and OPFS directory handles?
*
* @param dirHandle
*/
export async function looksLikePlaygroundDirectory(
dirHandle: FileSystemDirectoryHandle
) {
// Run this loop just to trigger an exception if the directory handle is no good.
// eslint-disable-next-line @typescript-eslint/no-unused-vars
for await (const _ of dirHandle.keys()) {
break;
}

try {
/**
* Assume it's a Playground directory if these files exist:
* - wp-config.php
* - wp-content/database/.ht.sqlite
*/
await dirHandle.getFileHandle('wp-config.php', { create: false });
const wpContent = await dirHandle.getDirectoryHandle('wp-content', {
create: false,
});
const database = await wpContent.getDirectoryHandle('database', {
create: false,
});
await database.getFileHandle('.ht.sqlite', { create: false });
} catch (e) {
return false;
}
return true;
}
51 changes: 5 additions & 46 deletions packages/playground/website/src/lib/state/redux/boot-site-client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ import { Blueprint, StepDefinition } from '@wp-playground/blueprints';
import { logger } from '@php-wasm/logger';
import { setupPostMessageRelay } from '@php-wasm/web';
import { startPlaygroundWeb } from '@wp-playground/client';
import { PlaygroundClient } from '@wp-playground/remote';
import {
PlaygroundClient,
looksLikePlaygroundDirectory,
} from '@wp-playground/remote';
import { getRemoteUrl } from '../../config';
import { setActiveModal, setActiveSiteError } from './slice-ui';
import { PlaygroundDispatch, PlaygroundReduxState } from './store';
Expand Down Expand Up @@ -72,7 +75,7 @@ export function bootSiteClient(
let isWordPressInstalled = false;
if (mountDescriptor) {
try {
isWordPressInstalled = await playgroundAvailableInOpfs(
isWordPressInstalled = await looksLikePlaygroundDirectory(
await directoryHandleFromMountDevice(mountDescriptor.device)
);
} catch (e) {
Expand Down Expand Up @@ -207,47 +210,3 @@ export function bootSiteClient(
signal.onabort = null;
};
}

/**
* Check if the given directory handle directory is a Playground directory.
*
* @TODO: Create a shared package like @wp-playground/wordpress for such utilities
* and bring in the context detection logic from wp-now – only express it in terms of
* either abstract FS operations or isomorphic PHP FS operations.
* (we can't just use Node.js require('fs') in the browser, for example)
*
* @TODO: Reuse the "isWordPressInstalled" logic implemented in the boot protocol.
* Perhaps mount OPFS first, and only then check for the presence of the
* WordPress installation? Or, if not, perhaps implement a shared file access
* abstraction that can be used both with the PHP module and OPFS directory handles?
*
* @param dirHandle
*/
export async function playgroundAvailableInOpfs(
dirHandle: FileSystemDirectoryHandle
) {
// Run this loop just to trigger an exception if the directory handle is no good.
// eslint-disable-next-line @typescript-eslint/no-unused-vars
for await (const _ of dirHandle.keys()) {
break;
}

try {
/**
* Assume it's a Playground directory if these files exist:
* - wp-config.php
* - wp-content/database/.ht.sqlite
*/
await dirHandle.getFileHandle('wp-config.php', { create: false });
const wpContent = await dirHandle.getDirectoryHandle('wp-content', {
create: false,
});
const database = await wpContent.getDirectoryHandle('database', {
create: false,
});
await database.getFileHandle('.ht.sqlite', { create: false });
} catch (e) {
return false;
}
return true;
}
Loading