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

Blueprints: Directory Resources #1793

Merged
merged 15 commits into from
Oct 7, 2024
Merged

Conversation

adamziel
Copy link
Collaborator

@adamziel adamziel commented Sep 22, 2024

Description

Adds a Directory resource type to enable loading file trees from git repositories, local hard drive, deeply nested zip archives etc:

{
	"steps": [
		{
			"step": "installPlugin",
			"pluginData": {
				"resource": "git:directory",
				"repositoryUrl": "https://github.com/WordPress/wordpress-playground.git",
				"ref": "HEAD",
				"path": "packages/docs"
			}
		},
		{
			"step": "installPlugin",
			"pluginData": {
				"resource": "literal:directory",
				"name": "hello-world",
				"files": {
					"README.md": "Hello, World!",
					"index.php": "<?php\n/**\n* Plugin Name: Hello World\n* Description: A simple plugin that says hello world.\n*/",
				}
			}
		}
	]
}

Motivation

This PR opens the door to:

  • Seamless Git integration. Import path mapping from git to Playground is now just a few steps referencing specific git directories. No more custom logic required!
  • Blueprint-based site imports and exports without any Playground webapp-specific logic.
  • Runtime-specific "resource overrides", e.g. --resource-override=GUTENBERG:./gutenberg.zip in CLI to test a Blueprint with a my local version of Gutenberg. The same logic would be used by the Blueprints builder to use files selected via <input type="file"> controls.

Schema

Every step can declare which kinds of resources it accepts (file-based resources vs directory-based resources). Using a single pluginData property in the installPlugin step means less choices for the developer. It also makes local resource overrides easy, e.g. we could tell Playground CLI to load a local Gutenberg directory instead of a remote Gutenberg zip. This wouldn't be as easy had we used separate options for passing ZIP-based and directory-based resources.

On one hand, pluginData is less informative than pluginZipFile. On the other, the name accommodates for non-zip resources such as directories.

Developer notes about specific API changes introduced in this PR

This PR changes introduces a new literal:directory resource that can be used in Blueprints as follows:

{
	"steps": [
		{
			"step": "installPlugin",
			"pluginData": {
				"resource": "literal:directory",
				"name": "hello-world",
				"files": {
					"README.md": "Hello, World!",
					"index.php": "<?php\n/**\n* Plugin Name: Hello World\n* Description: A simple plugin that says hello world.\n*/",
				}
			}
		}
	]
}

Or via the JS API:

await installTheme(php, {
	themeData: {
		name: 'test-theme',
		files: {
			'index.php': `/**\n * Theme Name: Test Theme`,
		},
	},
	ifAlreadyInstalled: 'overwrite',
	options: {
		activate: false,
	},
});

It also introduces a new writeFiles step:

{
	"steps": [
		{
			"step": "writeFiles",
			"writeToPath": "/wordpress/wp-content/plugins/my-plugin",
			"filesTree": {
				"name": "my-plugin",
				"files": {
					"index.php": "<?php echo '<a>Hello World!</a>'; ?>",
					"public": {
						"style.css": "a { color: red; }"
					}
				}
			}
		}
	]
}

Specific changes:

  • Adds a Resource<Directory> resource type that provides a name: string and files: FileTree.
  • Renames pluginZipFile to pluginData in the installPlugin step
  • Renames themeZipFile to themeData in the installPlugin step
  • Adds a new writeFiles step for writing entire directory trees
  • Adds a new literal:directory resource type where an entire file tree can be specified inline
  • Adds a new git:directory resource type that throws an error for now, but will load arbitrary directories from git repositories once Native git support: lsRefs(), sparseCheckout(), GitPathControl #1764 lands

Remaining work

  • Discuss the scope and the ideas
  • Add unit tests
  • Update the documentation
  • Adjust the installPlugin and installTheme step for compatibility with it's former signature. Ensure the existing packages consuming those functions from the @wp-playground/blueprints package will continue to work.
  • Confirm we can safely omit streaming from the system design at this point without setting ourselves up for a grand refactor a few months down the road.
    • I think we can! Streaming support could be an addition to the system, not a change in how the system works. For example, there could be a new DirectoryStream resource type producing an AsyncDirectoryIterator with streamable File or Blob objects as its leafs. It would work nicely with remote APIs or the ZIP streaming plumbing in @php-wasm/stream-compression. Any existing code expecting a DirectoryResource should be relatively easily adaptable to use these async iterators instead.

Follow-up work

  • Include actual git support once the Git sparse checkout PR lands
  • Ship a Playground CORS proxy to enable using git checkout in the webapp
  • Once we have a use-able git:directory resource, expand the developer notes from this PR and other related PRs and write a post on https://make.wp.org/playground

Tangent – Streaming and a shorthand URL notation

Without streaming, the entire directory must be loaded into memory. Our git sparse checkout implementation buffers everything anyway, but we will want to stream-read directory resources in the future. For example:

{
	"steps": [
		// Stream plugin files directly from a doubly zipped Git artifact
		{
			"step": "installPlugin",
			"pluginData": {
				"resource": "zip:github-artifact",
				"zipFile": {
					"resource": "url",
					"url": "https://github.com/WordPress/guteneberg/pr/54713/artifacts/build.zip"
				}
			}
		}
	}
}

That's extremely verbose, I'd love to explore a shorthand notation. One idea would be to make it a valid URI shaped after the data URL syntax:

const dataUri = `data:text/html;base64,%3Cscript%3Ealert%28%27hi%27%29%3B%3C%2Fscript%3E`;
const githubArtifactUri = `zip-github-artifact+url:https://github.com/WordPress/gutenberg/pr/54713/artifacts/build.zip`;
const gitResourceUri = `git:branch=HEAD;path=src,https://github.com/WordPress/hello-dolly.git`;

It wouldn't allow easy composition of the resources, e.g. a directory inside a zip sourced from a GitHub repo. Maybe that's for the best, though, since such a string would be extremely dense and difficult for humans to read. The object-based syntax might still be the most convenient way of to declare those.

@adamziel adamziel added [Package][@wp-playground] Blueprints [Type] Exploration An exploration that may or may not result in mergable code labels Sep 22, 2024
@adamziel adamziel marked this pull request as ready for review September 23, 2024 10:33
@adamziel adamziel linked an issue Sep 23, 2024 that may be closed by this pull request
@adamziel adamziel requested a review from a team as a code owner September 23, 2024 11:22
@akirk
Copy link
Member

akirk commented Sep 23, 2024

I think the proposal is the right call to make as a step for evolving the plugin file source. I am not sure I am sold on the data-URL like syntax although I don't have a better proposal at this point.

Regarding backwards compatibility: I think at this point there are already a lot of blueprints in circulation that use pluginZipFile, so I believe we need to continue to support this legacy syntax.

@adamziel
Copy link
Collaborator Author

Regarding backwards compatibility: I think at this point there are already a lot of blueprints in circulation that use pluginZipFile, so I believe we need to continue to support this legacy syntax.

Absolutely

@adamziel adamziel changed the title Explore a new Resource kind: Directory resources Blueprints: Directory Resources Sep 23, 2024
@adamziel adamziel added [Type] Enhancement New feature or request and removed [Type] Exploration An exploration that may or may not result in mergable code labels Sep 23, 2024
@adamziel
Copy link
Collaborator Author

Technically this PR is ready for merging – the build errors are related to #1766. Practically, I'll wait a bit more for feedback on the API shape.

Copy link
Collaborator

@bgrgicak bgrgicak left a comment

Choose a reason for hiding this comment

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

Great PR description!

@adamziel adamziel force-pushed the install-plugin-accept-directory branch from 84c713d to a77ce5a Compare October 7, 2024 17:59
@adamziel adamziel merged commit 1035a25 into trunk Oct 7, 2024
9 checks passed
@adamziel adamziel deleted the install-plugin-accept-directory branch October 7, 2024 19:29
adamziel added a commit that referenced this pull request Oct 7, 2024
Related to #1787, Follows up on #1793

Implements GitDirectoryResource to enable loading files directly from
git repositories as follows:

```ts
{
	"landingPage": "/guides/for-plugin-developers.md",
	"steps": [
		{
			"step": "writeFiles",
			"writeToPath": "/wordpress/guides",
			"filesTree": {
				"resource": "git:directory",
				"url": "https://github.com/WordPress/wordpress-playground.git",
				"ref": "trunk",
				"path": "packages/docs/site/docs/main/guides"
			}
		}
	]
}
```

 ## Implementation details

Uses git client functions merged in
#1764 to sparse
checkout the requested files. It also leans on the PHP CORS proxy which
is now started as a part of the `npm run dev` command.

The CORS proxy URL is configurable per `compileBlueprint()` call so that each
Playground runtime may choose to either use it or not. For example, it
wouldn't be very useful in the CLI version of Playground.

 ## Testing plan

Go to
`http://localhost:5400/website-server/#{%20%22landingPage%22:%20%22/guides/for-plugin-developers.md%22,%20%22steps%22:%20[%20{%20%22step%22:%20%22writeFiles%22,%20%22writeToPath%22:%20%22/wordpress/guides%22,%20%22filesTree%22:%20{%20%22resource%22:%20%22git:directory%22,%20%22url%22:%20%22https://github.com/WordPress/wordpress-playground.git%22,%20%22ref%22:%20%22trunk%22,%20%22path%22:%20%22packages/docs/site/docs/main/guides%22%20}%20}%20]%20}`
and confirm Playground loads a markdown file.
@adamziel adamziel mentioned this pull request Oct 7, 2024
adamziel added a commit that referenced this pull request Oct 8, 2024
Related to
#1787, Follows
up on #1793

Implements GitDirectoryResource to enable loading files directly from
git repositories as follows:

```ts
{
	"landingPage": "/guides/for-plugin-developers.md",
	"steps": [
		{
			"step": "writeFiles",
			"writeToPath": "/wordpress/guides",
			"filesTree": {
				"resource": "git:directory",
				"url": "https://github.com/WordPress/wordpress-playground.git",
				"ref": "trunk",
				"path": "packages/docs/site/docs/main/guides"
			}
		}
	]
}
```

 ## Implementation details

Uses git client functions merged in
#1764 to sparse
checkout the requested files. It also leans on the PHP CORS proxy which
is now started as a part of the `npm run dev` command.

The CORS proxy URL is configurable per `compileBlueprint()` call so that
each Playground runtime may choose to either use it or not. For example,
it wouldn't be very useful in the CLI version of Playground.

 ## Testing plan

Go to

```
http://localhost:5400/website-server/#{%20%22landingPage%22:%20%22/guides/for-plugin-developers.md%22,%20%22steps%22:%20[%20{%20%22step%22:%20%22writeFiles%22,%20%22writeToPath%22:%20%22/wordpress/guides%22,%20%22filesTree%22:%20{%20%22resource%22:%20%22git:directory%22,%20%22url%22:%20%22https://github.com/WordPress/wordpress-playground.git%22,%20%22ref%22:%20%22trunk%22,%20%22path%22:%20%22packages/docs/site/docs/main/guides%22%20}%20}%20]%20}
```

And confirm the Playground loads a markdown file.
adamziel added a commit that referenced this pull request Oct 8, 2024
#1793 introduced a directory resource type and changed the name of a few required parameters. Among others, the `themeZipFile` parameter in the `installTheme`  changed to `themeData`. Unfortunately, the `compile()` function had a type and did not backfill the new required parameter using the older, well known `themeZipFile` parameter which caused Blueprint execution errors.
adamziel added a commit that referenced this pull request Oct 8, 2024
Follows up on d0cec49 to fix the check in an `if` statement. More context: #1793 introduced a directory resource type and changed the name of a few required parameters. Among others, the `themeZipFile` parameter in the `installTheme`  changed to `themeData`. Unfortunately, the `compile()` function had a type and did not backfill the new required parameter using the older, well known `themeZipFile` parameter which caused Blueprint execution errors.
@asirota
Copy link

asirota commented Oct 23, 2024

Excellent improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Blueprints: Add a git Resource type
4 participants