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

Add faust blockset command #1585

Merged
merged 108 commits into from
Oct 18, 2023
Merged

Add faust blockset command #1585

merged 108 commits into from
Oct 18, 2023

Conversation

josephfusco
Copy link
Member

@josephfusco josephfusco commented Sep 20, 2023

Tasks

  • I have signed a Contributor License Agreement (CLA) with WP Engine.
  • If a code change, I have written testing instructions that the whole team & outside contributors can understand.
  • I have written and included a comprehensive changeset to properly document the changes I've made.

Description

These changes introduce a new command, faust blockset, for the @faustwp/cli package as well as the corresponding WP REST API endpoint to handle moving the blocks from the Next.js application into the WordPress uploads directory.

Related Issue(s):

Testing

  1. Pull down these changes: nojira-add-blockset-command. Run npm i and npm run build at root of monorepo.
  2. Run npm run blockset -w @faustwp/block-support-example from the root of the monorepo - this assumes you have plugins/faustwp/ symlinked into a local WordPress site.
  3. Observe that wp-content/uploads/faustwp/blocks/ in your WordPress site now contains block-a & block-b.
  4. Delete block-a from the Next.js application
  5. Run npm run blockset -w @faustwp/block-support-example again
  6. Observe that wp-content/uploads/faustwp/blocks/ now only contains block-b.

Demo

blockset-filetransfer-demo.mov

Signed-off-by: Joe Fusco <[email protected]>
Signed-off-by: Joe Fusco <[email protected]>
Signed-off-by: Joe Fusco <[email protected]>
Signed-off-by: Joe Fusco <[email protected]>
Signed-off-by: Joe Fusco <[email protected]>

# Conflicts:
#	package-lock.json
Signed-off-by: Joe Fusco <[email protected]>
Signed-off-by: Joe Fusco <[email protected]>
Signed-off-by: Joe Fusco <[email protected]>
Signed-off-by: Joe Fusco <[email protected]>
@changeset-bot
Copy link

changeset-bot bot commented Sep 20, 2023

🦋 Changeset detected

Latest commit: b641ca8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@faustwp/cli Minor
@faustwp/wordpress-plugin Minor
@faustwp/experimental-app-router Major
@faustwp/block-support-example Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Signed-off-by: Joe Fusco <[email protected]>
Signed-off-by: Joe Fusco <[email protected]>
Signed-off-by: Joe Fusco <[email protected]>
Signed-off-by: Joe Fusco <[email protected]>
Signed-off-by: Joe Fusco <[email protected]>
Signed-off-by: Joe Fusco <[email protected]>
Signed-off-by: Joe Fusco <[email protected]>
@blakewilson
Copy link
Contributor

Running the blockset command succeeded, but also I was asked this:

Screenshot 2023-10-13 at 11 44 48 AM

Will this be an issue for CI/CD environments?

@TeresaGobble
Copy link
Contributor

Nice work on this @josephfusco and @theodesp! Could we style Block B a little (like we have for block A) for the editor? When added and in save mode, it's not detectable:

Screen.Recording.2023-10-13.at.2.16.56.PM.mov

@TeresaGobble
Copy link
Contributor

Running the blockset command succeeded, but also I was asked this:

Will this be an issue for CI/CD environments?

Can confirm I saw this webpack-cli dependency prompt as well.

@theodesp
Copy link
Member

Running the blockset command succeeded, but also I was asked this:
Will this be an issue for CI/CD environments?

Can confirm I saw this webpack-cli dependency prompt as well.

we don't bundle @wordpress-scripts as a dependency because we ask the user to install it on their project. We could force to install the webpack as dev dependency for faust-cli package which will probably fix that.

@theodesp
Copy link
Member

Nice work on this @josephfusco and @theodesp! Could we style Block B a little (like we have for block A) for the editor? When added and in save mode, it's not detectable:

Screen.Recording.2023-10-13.at.2.16.56.PM.mov

@TeresaGobble the block is there I think the styles make it disappear. I will update the text color.

Screenshot 2023-10-16 at 11 14 13

@github-actions
Copy link
Contributor

github-actions bot commented Oct 16, 2023

📦 Next.js Bundle Analysis for @faustwp/getting-started-example

This analysis was generated by the Next.js Bundle Analysis action. 🤖

⚠️ Global Bundle Size Increased

Page Size (compressed)
global 245.33 KB (🟡 +35.49 KB)
Details

The global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

If you want further insight into what is behind the changes, give @next/bundle-analyzer a try!

New Page Added

The following page was added to the bundle from the code in this PR:

Page Size (compressed) First Load % of Budget (350 KB)
/example 843 B 246.15 KB 70.33%

@blakewilson
Copy link
Contributor

@josephfusco @theodesp When adding blocks to the home page, they are not displayed on the headless frontend. Do we need to import the proper GraphQL fragments (i.e. ${blocks.CreateBlockBlockB.fragments.entry} and ...${blocks.CreateBlockBlockB.fragments.key}) in the example project?

@blakewilson
Copy link
Contributor

Did a code review and everything looks good. I think we just need to expose the Block A & B to the front-page.js template so they are rendered when added to the home page in WordPress.

cc @josephfusco @theodesp

@theodesp
Copy link
Member

Did a code review and everything looks good. I think we just need to expose the Block A & B to the front-page.js template so they are rendered when added to the home page in WordPress.

cc @josephfusco @theodesp

Will add the fragments.

@theodesp
Copy link
Member

Styles fixed:

Headless
Screenshot 2023-10-17 at 12 16 18

Editor
Screenshot 2023-10-17 at 12 16 05

Screenshot 2023-10-17 at 12 15 58

@@ -23,7 +23,6 @@ jobs:
matrix:
next-dir: [
'examples/next/faustwp-getting-started',
'examples/next/block-support'
Copy link
Member

Choose a reason for hiding this comment

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

Adding this one is problematic.

@@ -11,3 +11,5 @@
@import '@wordpress/base-styles/breakpoints';
@import '@wordpress/block-library/src/style';
@import '@wordpress/block-library/src/theme';

@import '../wp-blocks/block-b/style.scss'
Copy link
Member

Choose a reason for hiding this comment

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

include block style here is next.js hates importing global styles within the components.

Copy link
Contributor

@blakewilson blakewilson left a comment

Choose a reason for hiding this comment

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

LGTM! Nice work @theodesp @josephfusco

@theodesp theodesp merged commit c29f83d into canary Oct 18, 2023
13 checks passed
@theodesp theodesp deleted the nojira-add-blockset-command branch October 18, 2023 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: @faustwp/cli The issue relates to CLI package package: faustwp Related to the companion WordPress plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants