Skip to content

fix: PluginSlot props are now required and tests are added - #157

Merged
raven-wing merged 5 commits into
Problematy:mainfrom
raven-wing:plugin_test_added
May 13, 2026
Merged

raven-wing merged 5 commits into
Problematy:mainfrom
raven-wing:plugin_test_added

Conversation

@raven-wing

@raven-wing raven-wing commented May 12, 2026 •

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • Bug Fixes

    • Plugin components now render correctly when property data is missing.
    • Plugin slots now require explicit properties instead of relying on implicit defaults.
  • Tests

    • Added tests verifying plugin slot behavior for registered and unregistered scopes.
    • Added test coverage for rendering scoped field labels with incomplete data states.

Review Change Stack

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 12, 2026 •

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

PluginSlot now requires a props object in its propTypes and no longer provides a defaultProps fallback. Unit tests verify behavior for unregistered and registered plugin scopes; an integration test confirms LocationDetailsBox renders plugin-scoped field labels even without plugin data.

Changes

PluginSlot Props Handling

Layer / File(s) Summary
PropTypes required, defaultProps removed
src/plugins/PluginSlot.jsx
PluginSlot.propTypes.props changed to PropTypes.object.isRequired and PluginSlot.defaultProps = { props: {} } was removed.
PluginSlot unit tests
tests/plugins/PluginSlot.test.jsx
New tests check that an unregistered scope yields an empty element and that a registered plugin component is rendered with the supplied props.
LocationDetailsBox plugin field integration
tests/MarkerPopup/LocationDetailsBox.test.jsx
Integration test verifies plugin-scoped field labels (e.g., promocode) render even when plugin data is not present.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

🐰 I hop to the slot, props in my paw,
Now required and tidy, no blank default flaw.
Tests hum like carrots beneath the moonlight,
Labels still show when plugins take flight.
Hooray — the map stays steady and bright.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title accurately summarizes the main changes: making PluginSlot props required and adding comprehensive tests for the component.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
tests/plugins/PluginSlot.test.jsx (1)

14-21: ⚡ Quick win

Add a regression test for omitted props on a registered plugin.

The new behavior in PluginSlot is defaulting missing props to {}, but current tests only cover explicit props values.

Proposed test addition
 describe('PluginSlot', () => {
@@
     it('renders the registered component with given props', () => {
         const TestComponent = ({ message }) => <span>{message}</span>;
         TestComponent.propTypes = { message: PropTypes.string.isRequired };
         act(() => registerPlugin('test-scope', TestComponent));

         render(<PluginSlot scope="test-scope" props={{ message: 'hello plugin' }} />);
         expect(screen.getByText('hello plugin')).toBeInTheDocument();
     });
+
+    it('does not crash when registered plugin is rendered without props', () => {
+        const TestComponent = () => <span>plugin without props</span>;
+        act(() => registerPlugin('test-scope-no-props', TestComponent));
+
+        expect(() => render(<PluginSlot scope="test-scope-no-props" />)).not.toThrow();
+        expect(screen.getByText('plugin without props')).toBeInTheDocument();
+    });
 });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/plugins/PluginSlot.test.jsx` around lines 14 - 21, Add a regression
test that verifies PluginSlot defaults missing props to {} by registering a
TestComponent (e.g., TestComponent used in existing test) and rendering
<PluginSlot scope="test-scope" /> with no props passed; assert the component
still renders (e.g., finds text or element) and does not crash — locate the
registration call registerPlugin('test-scope', TestComponent) and the PluginSlot
render invocation and add a new it/test case that omits the props prop to
confirm the new default behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/plugins/PluginSlot.test.jsx`:
- Around line 14-21: Add a regression test that verifies PluginSlot defaults
missing props to {} by registering a TestComponent (e.g., TestComponent used in
existing test) and rendering <PluginSlot scope="test-scope" /> with no props
passed; assert the component still renders (e.g., finds text or element) and
does not crash — locate the registration call registerPlugin('test-scope',
TestComponent) and the PluginSlot render invocation and add a new it/test case
that omits the props prop to confirm the new default behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 63fa63cf-d2fa-495e-a81e-671cb81c277e

📥 Commits

Reviewing files that changed from the base of the PR and between 5cf6ec3 and 7b0c41d.

📒 Files selected for processing (3)
  • src/plugins/PluginSlot.jsx
  • tests/MarkerPopup/LocationDetailsBox.test.jsx
  • tests/plugins/PluginSlot.test.jsx

@sonarqubecloud

Copy link
Copy Markdown

@raven-wing raven-wing changed the title fix: updated versions and added test fix: make PluginSlot props required and add tests May 13, 2026
@raven-wing raven-wing changed the title fix: make PluginSlot props required and add tests fix: PluginSlot props are now required and tests are added May 13, 2026
@raven-wing
raven-wing merged commit 5e6b24c into Problematy:main May 13, 2026
5 checks passed
@raven-wing
raven-wing deleted the plugin_test_added branch May 13, 2026 07:11
problematy-releaser Bot pushed a commit that referenced this pull request May 13, 2026
## [1.6.1](1.6.0...1.6.1) (2026-05-13)

### Bug Fixes

* PluginSlot props are now required and tests are added ([#157](#157)) ([5e6b24c](5e6b24c))
@problematy-releaser

Copy link
Copy Markdown

🎉 This PR is included in version 1.6.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant