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

Blueprint Builder #773

Merged
merged 39 commits into from
Dec 6, 2023
Merged

Blueprint Builder #773

merged 39 commits into from
Dec 6, 2023

Conversation

seanmorris
Copy link
Contributor

@seanmorris seanmorris commented Nov 22, 2023

What is this PR doing?

This PR adds the Blueprint Builder to the playground repo, to be served from playground.wordpress.net under the path /builder/builder.html

This PR also imports the client library to be served from playground.wordpress.net.
https://github.com/WordPress/wordpress-playground/pull/773/files#diff-34bb410e41a181987ce50ed5bbfc11bddfc830fdd514c98ac8dc1a072e9dd778R19

This PR also corrects the errant paths in @php-wasm/web:
https://github.com/WordPress/wordpress-playground/pull/773/files#diff-b9f9449aad40f58f9a350e7e4a0ed52a2ca2480e197b2b587e9d5b581a40d7a0L60

What problem is it solving?

This allows users to build & test blueprint JSON with a tight, visual feedback loop, right in their browser. It provides autocomplete to allow users to build blueprints quickly and efficiently, without having to shift back and forth between the editor & the documentation.

Testing Instructions

Navigate to the root of wordpress-playground, edit the file located at packages/playground/website/builder/builder.js. Replace lines 1-6 with the following:

const importStartPlaygroundWeb = import('//localhost:7001/client/index.js');
const fetchBlueprintSchema = fetch(
	'//localhost:7001/blueprints/blueprint-schema.json'
).then((r) => r.json());

Once that file is modified, run this to build the frontend:

nx reset
nx run playground-website:build:wasm-wordpress-net

Then run the following commands to start the dev servers:

pushd dist/packages/playground/wasm-wordpress-net/
http-server --port 7000 . &
popd
pushd dist/packages/playground/
http-server --port 7001 . --cors &
popd

Then navigate to http://localhost:7000/builder/builder.html to get started.

Protips: After running, you can manage the servers with the following commands:

  • You can use the jobs command to list any processes running in the current terminal's background.
  • You can use the fg command to bring the last process back to the terminal's foreground.
  • You can use fg %1, fg %2, fg %n... to bring a certain process back to the terminal's foreground.
  • You can use ctrl+z to pause a process
  • You can use bg to send a process to the terminal's background.
  • You can use kill %1, kill %2, kill %n... to kill a certain process.

That last one can be combined with kill -9 in sticky situations.

Implementation details:

Editor

We're currently using Ace editor for syntax highlighting and code editing. It makes things like autocomplete, displaying errors, and formatting really simple. We're exploring a move to codemirror upon iteration, however, due to Ace's less-than-wonderful mobile experience.

Partial JSON Parsing

For autocomplete, we can't guarantee that the current JSON stanza in the editor will actually be valid, since the user is currently typing! This PR ships with multiple functions geared toward partial parsing of JSON stanzas, regardless of whether or not they're actually syntactically valid.

Schema Dereferencer

This also ships with a Schema Reader, which is a proxy object that will return the properties of its underlying schema object transparently, but will automatically dereference $ref objects found inside the tree.

@adamziel
Copy link
Collaborator

adamziel commented Nov 23, 2023

To ship this on playground.wordpress.net, you'll also need to adjust the Vite build pipeline to ship the builder directory. Here's how it captures the demos directory:

rollupOptions: {
input: {
index: fileURLToPath(
new URL('./index.html', import.meta.url)
),
'sync.html': fileURLToPath(
new URL('./demos/sync.html', import.meta.url)
),
'peer.html': fileURLToPath(
new URL('./demos/peer.html', import.meta.url)
),
'time-traveling.html': fileURLToPath(
new URL('./demos/time-traveling.html', import.meta.url)
),
},

@adamziel
Copy link
Collaborator

I just found this issue: #793

@seanmorris seanmorris requested a review from a team as a code owner November 27, 2023 02:37
@adamziel
Copy link
Collaborator

adamziel commented Nov 28, 2023

@seanmorris I've pushed a few adjustments. I'll fix the lint issues and I think we'll be ready to merge. LMK what do you think.

One last item here would be to adjust the PR description to provide a bit more context like:

  • What is the rationale for this change? It's okay to copy some of the text from the related issue. Let's make this PR self-contained for anyone who comes here in the future.
  • What are some interesting implementation details? E.g. the ACE editor, the partial JSON parsing for autocompletion, ...
  • How to test it locally? It took me a while to figure out since the client is imported from the playground.wordpress.net URL that doesn't work at the moment.
  • What problems are there and what follow-up work might ensue? e.g. the mobile editing issues and exploring the CodeMirror path

Also, I created an issue to track the follow-up work: WordPress/blueprints#53

break;

case 'php':
list.push(...(await completePhpVersion(prefix)));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's ship this as is, but for posterity:

This approach will break sooner than later. It introduces tight coupling between the schema and the editor. However, we'll keep iterating on this schema. We'll add, remove, and change Blueprint properties every month and I would really like the editor to just keep working without any maintenance and housekeeping chores involved.

Let's ship this PR to give folks the editor they're waiting for and create a follow-up issue to decouple the schema from the editor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adamziel How can anything consume the schema if its "shape" is expected to change like that? Is there a way I can utilize it properly?

Copy link
Collaborator

@adamziel adamziel Nov 29, 2023

Choose a reason for hiding this comment

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

@seanmorris good question! VS Code seems to be able to autocomplete based on an arbitrary schema. My thinking is this:

  • We know "where" in the JSON we are at any given point based on the current input and the cursor position. For example, we know the top-level key (such as steps), the key below it (such as [2], and so on.
  • We know what the schema says about that path we're in. It says the top-level ref is "#/definitions/Blueprint",
  • We can follow that and see there's an object definition under definitions > Blueprint, and then we can see it has data under properties > steps which is an array of "#/definitions/StepDefinition".
  • We can resolve that ref to see what properties are expected there and how they're discriminated based on "propertyName": "step"

The only bit of custom logic here would be requesting the wp.org API to autocomplete the plugins and themes names.

Copy link
Contributor Author

@seanmorris seanmorris Nov 29, 2023

Choose a reason for hiding this comment

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

Copy link
Collaborator

@adamziel adamziel Nov 29, 2023

Choose a reason for hiding this comment

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

You're right, this PR ships generic functions that can read from specific places in that schema.

My point is that this PR also ships functions that are strongly tied to a specific structure – like that switch statement this thread is attached to where keys like wp, php, steps, and features are explicitly listed out and tied to schema-specific autocompletion functions like completePhpVersion() or completeSteps().

If the JSON schema changes, this program will require changes, too. At the same time, the same logic could be expressed in a way that can handle any JSON schema.

Once again, it's not a blocker here. Let's improve the description and ship this PR as is. The generic autocompletion engine will be useful in the future – let's track that work separately.

@seanmorris
Copy link
Contributor Author

seanmorris commented Dec 4, 2023

I just reviewed and here's what came up:

  • Would you remove all the changes unrelated to the builder UI? For example, this PR still ships updated php.wasm files that I mentioned in my previous comment.
  • Let's resolve merge conflicts and make sure the CI runs
  • Also, let's also document the following:
    • What are some interesting implementation details? E.g. the ACE editor, the partial JSON parsing for autocompletion, ...
    • What problems are there and what follow-up work might ensue? e.g. the mobile editing issues and exploring the CodeMirror path

@adamziel I've done most of that, but can I put the documentation that you've outlined in a P2?

@adamziel
Copy link
Collaborator

adamziel commented Dec 4, 2023

@adamziel I've done most of that, but can I put the documentation that you've outlined in a P2?

I think it would be more useful in this PR. Contributors who will want to learn more about this change will look for it here. Also, we'll need to keep track of the follow-up work here on GitHub so it makes sense to document it here.

Comment on lines 55 to 71

globalThis.File =
globalThis.File ||
class extends Blob {
lastModified = 0;
constructor(
bits: any,
name: string,
options: { type?: string; lastModified?: number }
) {
super(bits);
if (options.lastModified) {
this.lastModified = options.lastModified;
}
}
};

Copy link
Collaborator

@adamziel adamziel Dec 5, 2023

Choose a reason for hiding this comment

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

Is this change required by the Blueprints builder or can it be safely removed from this PR? Polyfilling the File class seems like a distinct issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adamziel I originally figured we could roll that update into this one, but you're right, we need to keep it separate. I've got another branch for that now.

const input = document.createElement('input');
input.setAttribute('type', 'file');
input.addEventListener('change', (event) => {
[...input.files].forEach((f) => {
Copy link
Collaborator

@adamziel adamziel Dec 6, 2023

Choose a reason for hiding this comment

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

Let's keep this as is to avoid delaying this PR anymore, but like @dmsnell and I mentioned in a few earlier rounds of feedback, let's avoid that destructuring syntax in general in favor of more readable patterns. In this case, this could be just input.files.

@adamziel
Copy link
Collaborator

adamziel commented Dec 6, 2023

This looks good to go. I will test the build process locally before merging just to be extra sure we're good, but other than that I think this PR is ready, thank you @seanmorris! 🎉

@adamziel
Copy link
Collaborator

adamziel commented Dec 6, 2023

I tested it locally and the build works nicely so I'll go ahead and merge. Yay! 🎉

Here's a few more notes about the testing instructions, though. The ones provided did not work for me.

For example, this sequence:

pushd dist/packages/playground/website/
http-server --port 7000 . &`
popd

Serves the website directory which where the remote.html file doesn't exist:

CleanShot 2023-12-06 at 13 44 47@2x

To work, this needs to serve the pushd dist/packages/playground/wasm-wordpress-net directory. To do that, this command can be reused: nx serve playground-website.

Also, http-server isn't a command that's available by default. Since that's an npm package, it's useful to prefix it with npx like so:

npx http-server --port 7000 . &

Also, as a general tip: when there are multiple processes to run, it's handy to say "do this in one terminal window, and do that in another terminal window". That way there's no need to manage background jobs, which can be inconvenient and requires additional mental models and commands.

That being said, I'm merging this PR. Thank you!

Comment on lines +107 to +119
<section id="ai">
<textarea
id="prompt"
placeholder="What kind of website do you want?"
></textarea>
<div class="toolbar">
<button id="close-ai" class="close"></button>
<span class="spacer"></span>
<button id="generate" class="generate">
GENERATE
</button>
</div>
</section>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Comment for posterity – let's remove this part of the HTML.

Copy link
Collaborator

@adamziel adamziel left a comment

Choose a reason for hiding this comment

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

Let's ship!

@adamziel adamziel merged commit 3a7db9c into WordPress:trunk Dec 6, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants