-
Notifications
You must be signed in to change notification settings - Fork 11
Conversation
Co-authored-by: Luis Herranz <luisherranz@users.noreply.github.com>
There is a way to copy PHP files to the build folder when using However, I think we can do better and detect |
I guess we should add something here. Is that correct, @gziolo? If so, any clue of what we need to add? |
It's probably a mix of changing the lines you highlighted and: My guess would be that we replace the lines I shared with (so probably inline the variable): const copyWebpackPatterns = '**/{block.json,*.php}'; Instead, we could use
|
Sounds good 👍 |
I have created this Pull Request to address it. I still want to review it but the code was working locally for me. |
@SantosGuillamot, thank you so much for opening this Pull Request. It's going to be a very nice enhancement. |
I'm having some issues reusing dynamic data (like Post Title or Post Date) in the server and the client using this method explained in the opening comment:
It seems that if the page is loaded quickly, the state defined in PHP and sent as an attribute is I've made the following video to show the issue I'm facing. If the page is loaded normally, it almost always returns the error. However, if I change the network to Fast 3G, it works fine. https://www.loom.com/share/08bf09f4a6434c3791d5d719fb5a2eb0 Any idea why this is happening or how this should be solved? |
I have no idea right now, but I'll try to debug it later. |
I have been debugging the issue with Mario and it looks like the It seems the Maybe we could implement something that depends on if (document.readyState === 'loading') { // Loading hasn't finished yet
document.addEventListener('DOMContentLoaded', doSomething);
} else { // `DOMContentLoaded` has already fired
doSomething();
} However, I don't understand why it is failing now in this branch and not before... 🤔 |
This is terrible. Definitely the worst part of the custom elements. The problem with const Show = ({ show, children }) => show && children; // children contains `<wp-block>` We need something more reliable. Jake Archibald talks about this problem in this video. He proposes:
@SantosGuillamot would you mind opening a new issue for this and mentioning it in the Custom Element's Tracking Issue so we can discuss the possibilities? |
You are totally right; I haven't thought of that problem.
This could work for subsequent EDIT: Or did you mean using both things at the same time? 😄 |
Done 🙂 I added it to the "Required to expose as experimental API in Gutenberg" section. Feel free to move it if it shouldn't be there. |
I don't know but let's move the discussion to the new issue 🙂 |
a26049a - nice work ❤️ I just double-checked, and the path like those used before would get ignored by webpack because they live outside of the source directory with blocks. So technically speaking, both ways would work. |
Hey @SantosGuillamot, is this still relevant? |
No, I think we can consider this experiment finished 🙂 |
I'm working on adding some dynamic blocks to better understand how it is the developer experience and the challenges we could face in the future.
The idea is to end up with really similar blocks to the ones we have right now but add something dynamic like the Post Title or Post Date. These are some of the steps/challenges I face prior to opening this Pull Request:
1. The
view.js
file wasn't being sent to the clientAt first, the
view.js
file was overwritten and it wasn't included in the build. Thanks to @DAreRodz , we figured out that we have to enqueue the script manually in thephp
file. Something like this:I also struggle to find the exact string we had to use but David helped me figure it out as well.
2. Reuse dynamic data from the server to the client
In these cases, we will probably need to be able to render the dynamic parts (like the Post Title or the Post Date) in the
view.js
. For doing that, we could use the REST API or add this data to the block attributes. We have used the second approach so far. At first, we started using a<span>
element with a class selector, but, if there are many attributes needed, @luisherranz suggested it could be cleaner to use something similar to "state":Previously with
<span>
Right now
3. The children and block title weren't save
The parent blocks, both the static and dynamic ones, add the possibility to edit the block title and add some children. However, at the beginning, I had some issues because in the dynamic ones this wasn't working until I figured out I had to add them in the
save
as well. In dynamic blocks, thesave
is not supposed to be needed, but it is for the editable content I believe.4. Start using
render.php
Once this Pull Request was merged, @luisherranz helped to start using the new
render
property. Previously we were using the traditionalindex.php
, but this new method is much cleaner.The only issue we found so far with the
render
way is that it does not bundle therender.php
files in the build directory, which means we had to point to the original source. Something likefile:../../../src/blocks/dynamic-interactive-child/render.php
.Bear in mind that this is still a WIP and I will keep improving the blocks and share the process here.