Clean up block implementation - #222
Merged
Merged
Conversation
Most of these blocks don't use this boilerplate file, which only had a console.log statement. In those cases, we remove the file entirely, as well as the entry in block.json which called for it to be loaded.
These blocks aren't part of a javascript application stack, so there is no need to declare an entry point into their materials.
The boilerplace declares wordpress dependencies to _always_ use the latest version, which causes problems in our CI workflow because a new release may have become available between when the developer started a branch and when the CI runs to evaluate it. Keeping branches short-lived is only a partial solution. The more robust option is to declare our dependencies using a predictable version. This provides for consistency between environments (among developers, and with our CI workflows). The cost of this is that we need to manage dependency updates ourselves, but this is already something we are doing.
The javascript community has a set of formatting rules which they've adopted, and their tooling will automatically apply that formatting. This commit applies the changes necessary for `npm run lint:js` to return no output. (We can tweak the rules that it applies as we go forward - some of these seem pretty silly to me - but having those discussions will take time to reflect and reach consensus, and we want to get this branch done quickly for the moment.
This is part of the NPM linter, but there were so many of these changes that it seemed better to split them out into their own commit.
Use of ' instead of ' causes complaints by the linter...
matt-bernhardt
force-pushed
the
cleanup
branch
2 times, most recently
from
September 14, 2026 15:13
33d863c to
12f9ca3
Compare
matt-bernhardt
marked this pull request as ready for review
September 14, 2026 15:15
The boilerplate included an import of useBlockProps - but then did not actually make use of it. Our choice is to either finish the implementation, or to stop importing it in the first place. I initially tried to actually implement this library for the blocks that are not rendered via render.php, but this resulted in editor-facing styles leaking out to the public UI. Instead, I've opted to remove the import entirely. It does not appear to impact how our defined options are updated via the editor interface, so I don't think this is something we need at this time.
The featured collection never passes any output through a translation function, so we do not need to import it in the first place.
This is the same fixup step for the PHP files as we previously took for the JS files. Run the linter, note what it complains about, and then fix those lines until the linter reports no output.
This keeps everything in sync with the latest releases of our tooling
matt-bernhardt
force-pushed
the
cleanup
branch
from
September 14, 2026 17:36
12f9ca3 to
773f5bb
Compare
matt-bernhardt
force-pushed
the
cleanup
branch
from
September 14, 2026 17:51
773f5bb to
bb20d8f
Compare
djanelle-mit
approved these changes
Sep 14, 2026
djanelle-mit
left a comment
djanelle-mit
left a comment
There was a problem hiding this comment.
Changes LGTM! I did not see any style issues on the rendered versions of the blocks, and the editable portions all work as expected.
The linting rules do seem a little persnickety to me, especially putting each attribute on it's own line, but happy to align on what we'd like those to do in a future chunk of work (after we wrap up this first feature branch).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This proposes a few cleanup steps to the blocks plugin materials:
view.js, the declaration of a"main"element inpackage.json, and certain dependencies like__anduseBlockPropsfor PHP-rendered blocks.npm run formatthat will handle a lot of simple things, and thennpm run lint:jswill flag more complex issues that need human hands. On the PHP side, there wasn't as much to do, socomposer lint web/app/plugins/mitlib-blocksshowed a handful of lines that needed changing.package-lock.jsonto make sure everything is in sync, and then a re-run of the build process to account for all of the above changes.My recommendation is to look at the commits one-by-one if you want to understand what is changing in a systematic way. Reviewing the overall changes is going to conflate a lot of things together, although that could be helpful if you're reading from the perspective of "can I continue to work with these files as-is?"