Bug: Fixing null/undefined theme argument for the WordPressBlocksProvider

[colinmurphy]

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

Fixed an issue with the theme argument for the WordPressBlockProvider to allow for null or undefined #1986

The condition to check if WordPressThemeContext is not needed as theme is an optional argument as per our docs - https://faustjs.org/reference/wordpressblocksprovider

e.g.

      <WordPressBlocksProvider
        config={{
          blocks,
          theme: null
        }}>
        <Component {...pageProps} key={router.asPath} />
      </WordPressBlocksProvider>

So setting the theme argument null, undefined or not at all should not result in an error.

How to replicate the issue

If you setup an Next.js app with Faust Blocks you should be able to replicate the error.

cd directory/
npx create-next-app@latest
cd my-app #assuming you set my-app as the project name
npm install @faustwp/blocks

src/pages/_app.js

import "@/styles/globals.css";
import { WordPressBlocksProvider } from "@faustwp/blocks";
import blocks from "../wp-blocks";


export default function App({ Component, pageProps }) {

  return (
    <WordPressBlocksProvider config={{ blocks, theme: undefined}}>
      <Component {...pageProps} />
    </WordPressBlocksProvider>
  );
}

src/pages/index.js

import { WordPressBlocksViewer } from "@faustwp/blocks";

export default function Home() {
  return (
    <div>
      <h1>Home</h1>
      <WordPressBlocksViewer blocks={
[
  {
    "name": "core/paragraph",
    "__typename": "CoreParagraph",
    "apiVersion": 3,
    "isDynamic": false,
    "attributes": {
      "cssClassName": null,
      "backgroundColor": null,
      "content": "A JavaScript framework that makes building headless WordPress simple and easy.",
      "style": null,
      "textColor": null,
      "fontSize": null,
      "fontFamily": null,
      "direction": null,
      "dropCap": false,
      "gradient": null,
      "align": null
    }
  },
  {
    "name": "core/paragraph",
    "__typename": "CoreParagraph",
    "apiVersion": 3,
    "isDynamic": false,
    "attributes": {
      "cssClassName": null,
      "backgroundColor": null,
      "content": "A familiar publisher experience, providing a cohesive experience with wp-admin. Easily extensible with built in filters, empowering developers to customize for different use cases.",
      "style": null,
      "textColor": null,
      "fontSize": null,
      "fontFamily": null,
      "direction": null,
      "dropCap": false,
      "gradient": null,
      "align": null
    }
  }
        ]
      } />
    </div>
  );
};

src/wp-blocks/index.js

import { CoreBlocks } from "@faustwp/blocks";

export default {
	...CoreBlocks,
};

if you run npm run dev you will get the following error:

useBlocksTheme hook was called outside of context, make sure your app is wrapped with WordPressBlocksProvider

The following PR fixes that issue.

Further Discussion

After debugging this issue with @moonmeister we also found that the theme argument appears to be redundant.
If you have a look at any of the blocks, it is only passed to the getStyles component which doesn't use the theme argument.

e.g.

https://github.com/wpengine/faustjs/blob/canary/packages/blocks/src/blocks/CoreParagraph.tsx
https://github.com/wpengine/faustjs/blob/canary/packages/blocks/src/utils/get-styles/getStyles.ts
https://github.com/wpengine/faustjs/blob/canary/packages/blocks/src/utils/get-styles/getTextStyles.ts#L10

There is a question on whether this is needed as none of the style components use this argument.

Related Issue(s):

#1986

Testing

As described above using a next.js app

Screenshots

Documentation Changes

Dependant PRs

[changeset-bot]

🦋 Changeset detected

Latest commit: 9059522

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

This PR includes changesets to release 1 package

Name	Type
@faustwp/blocks	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

[github-actions]

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

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

This PR introduced no changes to the JavaScript bundle! 🙌

[colinmurphy]

@moonmeister Can you review this please and see if you can still replicate issue #1986 please

[colinmurphy]

@moonmeister I need to update this PR as some of the logic isn't correct

[colinmurphy]

@moonmeister This is ready for review.

[moonmeister]

Can you explain why/how this fixes the issue?

[colinmurphy]

@moonmeister

What PR should solve is the issue around not allowing null for the theme argument for the WordPressBlockProvider. FWIW I couldn't ever replicate the bug but this is what the PR should do

1. Checks if both WordPressBlocksContext and WordPressThemeContext is defined, if not then it should throw an error (spotted a typo in the code)

2. Then it should allow WordPressThemeContext values to be null. It will only throw an error if undefined

[colinmurphy]

@moonmeister

Thank you for help on debugging this issue. I have updated the PR. I have also updated details on how to replicate the issue and also how it appears theme might be a redundant argument.

[moonmeister]

@colinmurphy I'm guessing we need to leave these tests in now that we've changed the fix, yes?

[colinmurphy]

@moonmeister We probably don't. I was leaving this for reference if someone wanted to use the theme argument but I removed the failing test for undefined.

[colinmurphy]

@moonmeister Tests reverted back. Thanks for pointing this out.

[colinmurphy]

Just confirming I did some further testing and the following 3 scenarios worked. I just wanted to test again as we will be updating the docs for Faust to remove the theme argument.

// src/pages/_app.js
import "@/styles/globals.css";
import { WordPressBlocksProvider } from "@faustwp/blocks";
import blocks from "../wp-blocks";
export default function App({ Component, pageProps }) {

  return (
    <WordPressBlocksProvider config={{ blocks}}>
      <Component {...pageProps} />
    </WordPressBlocksProvider>
  );
}

// src/pages/_app.js
import "@/styles/globals.css";
import { WordPressBlocksProvider } from "@faustwp/blocks";
import blocks from "../wp-blocks";
export default function App({ Component, pageProps }) {

  return (
    <WordPressBlocksProvider config={{ blocks, theme: null}}>
      <Component {...pageProps} />
    </WordPressBlocksProvider>
  );
}

// src/pages/_app.js
import "@/styles/globals.css";
import { WordPressBlocksProvider } from "@faustwp/blocks";
import blocks from "../wp-blocks";
export default function App({ Component, pageProps }) {

  return (
    <WordPressBlocksProvider config={{ blocks, theme: undefined}}>
      <Component {...pageProps} />
    </WordPressBlocksProvider>
  );
}

[josephfusco]

LGTM!