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

createContext, useContext from @wordpress/element doesn't work on InnerBlocks in custom block #50633

Open
EmranAhmed opened this issue May 15, 2023 · 7 comments
Labels
[Package] Scripts /packages/scripts [Type] Bug An existing feature does not function as intended

Comments

@EmranAhmed
Copy link

Description

I create two block name 'parent' and 'child' by npx @wordpress/create-block add context provider on parent block with value, but inner block child block cannot show parent block context values.

Step-by-step reproduction instructions

  1. cd /wp-content/plugins
  2. npx @wordpress/create-block --namespace=Custom --title="Test Context" context-test
  3. cd context-test/src
  4. open .
  5. mkdir context-parent
  6. mv * context-parent
  7. npx @wordpress/create-block --namespace=Custom --title="Context child" context-child --no-plugin
  8. touch utils.js
  9. cd context-parent
  10. Edit block.json name to "custom/context-parent"

Screenshots, screen recording, code snippet

Here is GitHub public repo link: https://github.com/EmranAhmed/gutenberg-context-issue

Environment info

  • WP 6.2
  • Chrome: 113.0.5672.92 (Official Build) (x86_64)
  • Theme: Twenty Twenty-ThreeVersion: 1.1
  • No other plugin activated

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@EmranAhmed EmranAhmed changed the title createContext, useContext from @wordpress/element doesn't work on InnerBlocks in custom block createContext, useContext from @wordpress/element doesn't work on InnerBlocks in custom block May 15, 2023
@talldan
Copy link
Contributor

talldan commented May 16, 2023

Here is GitHub public repo link: https://github.com/EmranAhmed/gutenberg-context-issue

Thanks for providing that. It'd be good if it could be installed as a plugin. At the moment it causes some errors.

I'm also not really sure I understand the "Step-by-step reproduction steps" that have been provided here. Could you provide some steps that someone could follow to reproduce the issue? Also sharing what the expected and actual outcome is would be good.

Contributors are very busy, so the best chance of success is to make it as easy as possible to reproduce the issue.

@talldan talldan added [Status] Needs More Info Follow-up required in order to be actionable. [Package] Element /packages/element labels May 16, 2023
@EmranAhmed
Copy link
Author

@talldan Reproduction is simple: Just create two block with @wordpress/create-block make a block wrapped with InnerBlocks with context provider, use another block as child block. And try to get context value passed from parent block in child block.

To check this repo: https://github.com/EmranAhmed/gutenberg-context-issue

Goto terminal and run

npm install

npm start

@talldan
Copy link
Contributor

talldan commented May 16, 2023

I was able to reproduce this. I'm not sure why it happens, but it looks like on the first render useContext only returns the default context value (the value when context is first defined, not the value of the value prop).

If the context provider's value is updated, the child doesn't re-render and so still shows the default context value.

useContext is the same implementation as react, so it's curious. The provided code looks correct to me.

My first thought was that maybe AsyncModeProvider was interfering with re-renders, but I removed that and no difference.

@kevin940726
Copy link
Member

I think it's a bug in @wordpress/scripts. Context is working as expected and child blocks are rendered under the parent's context provider too. The problem is that @wordpress/scripts by default bundles each block into its own build file. The output in the build folder roughly looks like:

build
├── context-child
│   ├── block.json
│   ├── index.asset.php
│   └── index.js
└── context-parent
    ├── block.json
    ├── index.asset.php
    └── index.js

Note that there is no common chunk so each generated index.js has its own copy of the context from utils.js.

The solution is to somehow split out the common chunk of context into its own file so that both blocks can share the same context. I don't think it can be easily done in @wordpress/scripts right now?

Another idea is to externalize the code for the context, which might be a bit easier.

The worst case is to define the context as a global variable, which is often seemed as an anti-pattern in most cases. Not sure if it applies here though.

@gziolo Do you have any suggestions for this issue? This looks counter-intuitive and isn't that easy to debug. We might want to do something about this in the build process somehow to prevent future similar confusion 🤔 .

@kevin940726 kevin940726 added [Type] Bug An existing feature does not function as intended [Package] Scripts /packages/scripts and removed [Status] Needs More Info Follow-up required in order to be actionable. [Package] Element /packages/element labels May 16, 2023
@gziolo
Copy link
Member

gziolo commented May 19, 2023

Here is GitHub public repo link: https://github.com/EmranAhmed/gutenberg-context-issue

Thank you for the report @EmranAhmed with the linked repository that helped identify the issue's source.

Note that there is no common chunk so each generated index.js has its own copy of the context from utils.js.

@kevin940726 was able to precisely identify the issue and offer some ways to address it without making changes to the @wordpress/scripts package. There are even more ways to resolve the issue:

  • Importing the files necessary for the editor from the context-child block in the parent-child block. In effect, you would only define editorScript inside block.json for the parent block. This way, there is going to be only a single bundle loaded, so everything should work as expected. I made the assumption that you always register all custom blocks by orchestrating everything with the parent block.
  • It's possible to override the webpack config as explained in Extending the webpack config. In this case, it requires creating a commons chunk, which includes all code shared between entry points. See Split Chunks: Example 1 in webpack documentation.

Another idea is to externalize the code for the context, which might be a bit easier.

@kevin940726, can you explain what would that approach require?

The worst case is to define the context as a global variable, which is often seemed as an anti-pattern in most cases. Not sure if it applies here though.

That would be similar to bundling both blocks together. Here, however, it would be the parent block that exposes the context behind the global variable:

omport { createContext } from '@wordpress/element'

export const Context = createContext({ id: 0 })

window.MyPluginParent.Context = Context;

Then in the child block:

const x = useContext(window.MyPluginParent.Context);

Do you have any suggestions for this issue? This looks counter-intuitive and isn't that easy to debug. We might want to do something about this in the build process somehow to prevent future similar confusion 🤔

It would be great to integrate The Manifest into the webpack config so there is a single file that keeps detailed notes on all the chunks and their dependencies required for a block. This would also allow us to offer some interesting improvements to the developer experience. It would be possible to register multiple blocks from several block.json files. At the moment, developers have to register every block separately. It would be possible to update register_block_type or add new register_block_types on the server to support auto-discovery of multiple blocks through block.json in a given target folder and its subdirectories. It all could be orchestrated through the manifest file generated by webpack, which would simplify everything behind the scenes.

By the way, not having a way to automatically enqueue the webpack runtime is the reason we still don't have React Fast Refresh working out of the box with multiple entry points (blocks) as explained in #25188 (comment).

@kevin940726
Copy link
Member

Another idea is to externalize the code for the context, which might be a bit easier.

@kevin940726, can you explain what would that approach require?

I don't have specific ideas in mind. I think roughly it would look like how we enqueue wp code: make a separate bundle just for the context and make those blocks require them by externalizing the dependency. This should look somehow very similar to the global variable approach though.

@jsnajdr
Copy link
Member

jsnajdr commented Nov 27, 2023

Looking at the provided example, I must say that this behavior is kind of expected, this is how JavaScript bundling works 🙁 The Parent and Child blocks are built separately, as independent scripts. That's a legitimate way how to build them, it's not a bug in @wordpress/scripts.

Both blocks are using (and bundling) the utils module, which contains the context definition, and each of them bundles their own copy. Then the two contexts are different.

We could configure webpack to create a shared chunk for utils. The optimization.splitChunks.chunks option would have to be set to all, and the splitChunks.minSize would have to be smaller than the default 20kB, so that the utils module is not considered too small to be worth optimizing. But this is an optional optimization, it's still legitimate for any bundler to just create two copies of utils, and therefore two contexts.

To make it work reliably, the context would have to live in its own library, its own WordPress script. The reference in the blocks would have to be externalized, and @wordpress/scripts would have to build the library as a script, with an window.wp.xyz = ... export and an .asset.php file. Currently @wordpress/scripts can build only blocks, not general-purpose libraries. That could be a nice addition.

But the very first thing you should look at is the Gutenberg block context API, which lets you propagage a selected attribute of a parent block to its children.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Scripts /packages/scripts [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

5 participants