-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Remove unneeded block.json name-only imports #53677
Conversation
const hasFootnotesBlockType = useSelect( | ||
( select ) => | ||
!! select( blocksStore ).getBlockType( 'core/footnotes' ), | ||
[] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here the code cares only whether the block is registered or not. Return a boolean from useSelect
should be slightly faster.
? [] | ||
: [ createBlock( listName, listAttributes, children ) ] | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This createListItem
function is unused. Nobody ever imports it from the utils.js
module, and it's not a part of public API either.
parentCount: getBlockParentsByBlockName( | ||
clientId, | ||
'core/navigation-submenu' | ||
).length, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Little refactoring in order to call the getBlockParentsByBlockName
just once.
const blockName = `${ queryLoopName }/${ activeVariationName }`; | ||
const activeVariationPatterns = useSelect( | ||
const blockName = `core/query/${ activeVariationName }`; | ||
const hasActiveVariationPatterns = useSelect( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here again the code is interested only in the boolean value (is the list non-empty) not in the actual list.
Size Change: -455 B (0%) Total Size: 1.51 MB
ℹ️ View Unchanged
|
Flaky tests detected in 8aa9b90. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5866602572
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice refactoring. Everything looks good.
Nice one 👍 |
Incremental step towards adopting standard JSON module imports, see also #34176 and #53470. This PR removes imports from
block.json
files that only care about the block name (e.g.,core/heading
).The code reads better (less indirection, better greppable) when the block names are hardcoded as strings. You will notice that virtually every modified file already references other block names as strings anyway.
Also, the JSON modules standard doesn't support named imports, only default ones. So, named import usages would need to be modified anyway, and here we solve the problem by removing them.
Because JSON imports are currently inlined by a Babel plugin, the
block-library
will get a bit smaller, now that fullblock.json
files are no longer inlined just to read the.name
field.The only remaining places that import
block.json
files are the block registration scripts.