Skip to content

feat(menu): subcomponents log console warning when used outside Menu (FE-7202) #7300

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

Merged
merged 4 commits into from
Apr 25, 2025

Conversation

Parsium
Copy link
Contributor

@Parsium Parsium commented Apr 11, 2025

Proposed behaviour

Ensure a console warning is logged when a Menu subcomponent is not rendered within a Menu.

For example, rendering:

<MenuItem href="#">Item</MenuItem>

will log the following:

Carbon Menu: Context not found. Have you wrapped your Carbon subcomponents properly? See stack trace for more details.
This logged warning will become a thrown error in a future major release.
  error @ index.ts:25
  useContext @ createStrictContext.ts:46
  MenuItem @ menu-item.component.tsx:174

Wrapping MenuItem in a Menu resolves the warning:

<Menu>
  <MenuItem href="#">Item</MenuItem>
</Menu>

Current behaviour

Menu and its subcomponents follow the Compound Component pattern—they use React context to share state. If a subcomponent is rendered outside of a Menu, no error handling occurs. This means developers using these components incorrectly won't receive any warning that they're following an anti-pattern.

Checklist

  • Commits follow our style guide
  • Related issues linked in commit messages if required
  • Screenshots are included in the PR if useful
  • All themes are supported if required
  • Unit tests added or updated if required
  • Playwright automation tests added or updated if required
  • Storybook added or updated if required
  • Translations added or updated (including creating or amending translation keys table in storybook) if required
  • Typescript d.ts file added or updated if required
  • Related docs have been updated if required

QA

  • Tested in provided StackBlitz sandbox/Storybook
  • Add new Playwright test coverage if required
  • Carbon implementation matches Design System/designs
  • UI Tests GitHub check reviewed if required

Additional context

For now, createStrictContext logs an error to the browser console and returns a fallback default value, as we are aware this anti-pattern has been used in practice, and we want to avoid breaking apps at this stage. This behaviour should be replaced with throwing an error in the future.

Testing instructions

@Parsium Parsium force-pushed the menu-stricter-context-handling branch 2 times, most recently from 5d76be9 to e345497 Compare April 14, 2025 15:36
edleeks87
edleeks87 previously approved these changes Apr 15, 2025
@Parsium Parsium force-pushed the menu-stricter-context-handling branch from e345497 to 511e663 Compare April 15, 2025 13:42
@Parsium Parsium changed the title feat(menu): subcomponents log console warning when used outside Menu feat(menu): subcomponents log console warning when used outside Menu (FE-7202) Apr 15, 2025
@DipperTheDan DipperTheDan self-requested a review April 17, 2025 13:01
DipperTheDan
DipperTheDan previously approved these changes Apr 17, 2025
@DipperTheDan DipperTheDan marked this pull request as ready for review April 17, 2025 14:18
@DipperTheDan DipperTheDan requested review from a team as code owners April 17, 2025 14:18
@Parsium Parsium dismissed stale reviews from DipperTheDan and edleeks87 via b8f185f April 24, 2025 14:27
@Parsium Parsium force-pushed the menu-stricter-context-handling branch from 511e663 to b8f185f Compare April 24, 2025 14:27
@Parsium Parsium merged commit a698b3e into master Apr 25, 2025
28 of 29 checks passed
@Parsium Parsium deleted the menu-stricter-context-handling branch April 25, 2025 10:11
@carbonci
Copy link
Collaborator

🎉 This PR is included in version 153.7.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

5 participants