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

Update Basic block. #148

Merged
merged 1 commit into from
Sep 22, 2021
Merged

Update Basic block. #148

merged 1 commit into from
Sep 22, 2021

Conversation

ryanwelcher
Copy link
Contributor

@ryanwelcher ryanwelcher commented Sep 22, 2021

This PR updates the 01-basic block to use a block.json file.

One potential issue I noticed is that because the block doesn't have a build process, no *.asset.php file is generated. This requires that it be added manually in order to have the block successfully registered via register_block_type and block.json.

It does beg the question, do we need to have an official approach for creating custom blocks that don't use build processes?

@ryanwelcher ryanwelcher requested review from mkaz and gziolo September 22, 2021 16:01
@mkaz
Copy link
Member

mkaz commented Sep 22, 2021

We don't have an official approach, most of the tooling and recommendation is to use the build process; since that is the way to add tooling and structure. If it is all just hand coded, there isn't a linter or other means to generate necessary files.

Also, most examples and all the Gutenberg source is written using JSX and uses a build step, so by embracing it gives more code to look at and learn from, without needing to translate. Another reason we strongly encourage using the build step.

Copy link
Member

@mkaz mkaz left a comment

Choose a reason for hiding this comment

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

This looks good, I tested it out and it all worked as expected no issues. 👍

One question on the dependencies is the wp-polyfill required? I suppose it depends on what the developer wants to support but for WordPress in general I believe we're trying to phase that out.

@ryanwelcher
Copy link
Contributor Author

One question on the dependencies is the wp-polyfill required? I suppose it depends on what the developer wants to support but for WordPress in general I believe we're trying to phase that out.

Not required no, it is added by default via the dependencyExtractionPlugin so I left it in.

@mkaz mkaz merged commit 49892fe into master Sep 22, 2021
@mkaz mkaz deleted the feature/update-example-01 branch September 22, 2021 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants