Remove ge tEditWrapperProps and update blockProps#6192
Remove ge tEditWrapperProps and update blockProps#6192kyrapieterse wants to merge 1 commit intomasterfrom
Conversation
📝 WalkthroughWalkthroughThese changes refactor how unique IDs are handled in maxi-block components. The Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/maxi-block/maxiBlock.js`:
- Around line 280-282: Remove the redundant uniqueid attribute from the saved
block props: in maxiBlock.js where blockProps is built (the spread ...(isSave &&
{ id: uniqueID, uniqueid: uniqueID }) pattern), stop including uniqueid:
uniqueID when isSave is true so only id: uniqueID remains; update the blockProps
construction that references uniqueID and isSave to omit the uniqueid key (leave
id: uniqueID intact).
| ...(isSave && { | ||
| id: uniqueID, | ||
| }), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if frontend JS references the 'uniqueid' HTML attribute (lowercase) for querying elements
rg -n --type=js --type=ts 'uniqueid' -g '!node_modules' -g '!*editor*' -g '!*maxi-block*' | head -30Repository: maxi-blocks/maxi-blocks
Length of output: 210
🏁 Script executed:
#!/bin/bash
# Check how uniqueid attribute is used in save/frontend context
rg -n 'querySelector.*uniqueid\|getAttribute.*uniqueid\|dataset.*uniqueid\|\[uniqueid' -g '!node_modules' | head -20Repository: maxi-blocks/maxi-blocks
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Examine the actual code at lines 280-282 with context
sed -n '270,290p' src/components/maxi-block/maxiBlock.jsRepository: maxi-blocks/maxi-blocks
Length of output: 469
🏁 Script executed:
#!/bin/bash
# Check the popover-button component to see how it handles the uniqueid prop
sed -n '110,140p' src/components/maxi-popover-button/index.jsRepository: maxi-blocks/maxi-blocks
Length of output: 466
🏁 Script executed:
#!/bin/bash
# Look for where uniqueid is rendered as an HTML attribute (check for spread operators or direct attr rendering)
rg -n -B 2 -A 2 'uniqueid' src/components/maxi-popover-button/index.jsRepository: maxi-blocks/maxi-blocks
Length of output: 213
🏁 Script executed:
#!/bin/bash
# Check the slider-maxi context for why uniqueid is removed
sed -n '200,225p' src/blocks/slider-maxi/edit.jsRepository: maxi-blocks/maxi-blocks
Length of output: 679
🏁 Script executed:
#!/bin/bash
# Check how uniqueid gets rendered as HTML attribute in the actual rendered output
# Look for where maxiBlock component renders its props
rg -n -B 5 -A 5 'return.*<div' src/components/maxi-block/maxiBlock.js | head -50Repository: maxi-blocks/maxi-blocks
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Search for any HTML rendering that uses the uniqueid prop
rg -n 'uniqueid=' src/components/maxi-block/ -A 2 -B 2Repository: maxi-blocks/maxi-blocks
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Check if there's any documentation or comments about uniqueid purpose
rg -n -i 'uniqueid.*comment\|unique.*id.*\(backward\|legacy\|compat\|frontend\)' src/components/maxi-block/Repository: maxi-blocks/maxi-blocks
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Get a broader view of the maxiBlock component - see the return statement and how props are rendered
sed -n '250,320p' src/components/maxi-block/maxiBlock.jsRepository: maxi-blocks/maxi-blocks
Length of output: 1352
🏁 Script executed:
#!/bin/bash
# Check the entire maxiBlock.js file structure to understand the component
wc -l src/components/maxi-block/maxiBlock.js && head -50 src/components/maxi-block/maxiBlock.jsRepository: maxi-blocks/maxi-blocks
Length of output: 1231
🏁 Script executed:
#!/bin/bash
# Look for where the component object is used - see if it's spread into HTML or used elsewhere
rg -n 'return\s*\(' src/components/maxi-block/maxiBlock.js -A 10 | head -40Repository: maxi-blocks/maxi-blocks
Length of output: 834
🏁 Script executed:
#!/bin/bash
# Check how MainMaxiBlock and InnerBlocksBlock handle the uniqueid and id props
rg -n -A 10 'function MainMaxiBlock\|const MainMaxiBlock\|export.*MainMaxiBlock' src/components/maxi-block/mainMaxiBlock.js | head -40Repository: maxi-blocks/maxi-blocks
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Check the structure of InnerBlocksBlock to see if it renders these as HTML attributes
rg -n -A 10 'function InnerBlocksBlock\|const InnerBlocksBlock\|export.*InnerBlocksBlock' src/components/maxi-block/innerBlocksBlock.js | head -40Repository: maxi-blocks/maxi-blocks
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Final check: verify if uniqueid appears in any saved output or serialization
rg -n 'uniqueid' src/components/maxi-block/mainMaxiBlock.js src/components/maxi-block/innerBlocksBlock.jsRepository: maxi-blocks/maxi-blocks
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Check what MainMaxiBlock actually renders - find its implementation
fd -t f mainMaxiBlock.js | xargs cat | head -100Repository: maxi-blocks/maxi-blocks
Length of output: 2248
🏁 Script executed:
#!/bin/bash
# Look for tagName usage - it appears blockProps.tagName is used to render
rg -n 'tagName' src/components/maxi-block/mainMaxiBlock.js -B 2 -A 2Repository: maxi-blocks/maxi-blocks
Length of output: 145
🏁 Script executed:
#!/bin/bash
# Check if the component spreads attributes as HTML attributes on a DOM element
cat src/components/maxi-block/mainMaxiBlock.js | head -80Repository: maxi-blocks/maxi-blocks
Length of output: 1799
🏁 Script executed:
#!/bin/bash
# Check what useBlockProps.save does with props
rg -n 'useBlockProps.save' --type=js --type=ts | head -20Repository: maxi-blocks/maxi-blocks
Length of output: 248
🏁 Script executed:
#!/bin/bash
# Verify that props containing uniqueid get passed to useBlockProps.save
rg -n 'uniqueid' src/components/maxi-block/maxiBlock.js -B 5 -A 5 | head -40Repository: maxi-blocks/maxi-blocks
Length of output: 319
🏁 Script executed:
#!/bin/bash
# Let's verify the WordPress useBlockProps behavior - check if props get spread as HTML attributes
rg -n 'import.*useBlockProps' src/components/maxi-block/mainMaxiBlock.jsRepository: maxi-blocks/maxi-blocks
Length of output: 125
🏁 Script executed:
#!/bin/bash
# Check if there's any filtering of props before they reach useBlockProps.save
sed -n '30,50p' src/components/maxi-block/mainMaxiBlock.jsRepository: maxi-blocks/maxi-blocks
Length of output: 523
🏁 Script executed:
#!/bin/bash
# Confirm that uniqueid in props reaches useBlockProps.save and becomes an HTML attribute
# Check WordPress documentation or look for how useBlockProps handles props
rg -n 'uniqueid.*HTML\|HTML.*attribute' src/ -iRepository: maxi-blocks/maxi-blocks
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Final check: see if uniqueid attribute appears in any saved markup or tests
rg -n 'uniqueid=' -g '*.html' -g '*.php' -g '*.test.js' -g '*.test.ts'Repository: maxi-blocks/maxi-blocks
Length of output: 49
Minor: uniqueid is redundant in save output and should be removed.
Both uniqueid and id are set to the same uniqueID value on save. Frontend JavaScript does not query elements using the uniqueid attribute, and there is evidence that uniqueid is being explicitly removed in other contexts (e.g., slider-maxi/edit.js:212). Remove uniqueid: uniqueID, from blockProps on line 271 to avoid unnecessary HTML attribute duplication in saved markup.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/maxi-block/maxiBlock.js` around lines 280 - 282, Remove the
redundant uniqueid attribute from the saved block props: in maxiBlock.js where
blockProps is built (the spread ...(isSave && { id: uniqueID, uniqueid: uniqueID
}) pattern), stop including uniqueid: uniqueID when isSave is true so only id:
uniqueID remains; update the blockProps construction that references uniqueID
and isSave to omit the uniqueid key (leave id: uniqueID intact).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a20f59d10
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ref, | ||
| id: uniqueID, | ||
| key: `maxi-block-${uniqueID}`, | ||
| uniqueid: uniqueID, |
There was a problem hiding this comment.
Prevent adding uniqueid to saved block markup
Setting uniqueid in shared blockProps makes every MaxiBlock.save path serialize a new wrapper attribute, because both MainBlock and MainInnerBlocksBlock forward ...props into useBlockProps.save(...). Existing posts were saved without this attribute, so this change alters the canonical save output and can trigger Gutenberg block validation failures (“unexpected or invalid content”) across blocks that use this component when old content is opened in the editor.
Useful? React with 👍 / 👎.
Fixes #3610
Summary by CodeRabbit