Skip to content

[utils] Fix potential EMPTY_OBJECT mutation in useRenderElement#3986

Draft
Jkker wants to merge 2 commits intomui:masterfrom
Jkker:fix/use-render-element-empty-object-mutation
Draft

[utils] Fix potential EMPTY_OBJECT mutation in useRenderElement#3986
Jkker wants to merge 2 commits intomui:masterfrom
Jkker:fix/use-render-element-empty-object-mutation

Conversation

@Jkker
Copy link

@Jkker Jkker commented Feb 5, 2026

Summary

This PR fixes a latent bug in useRenderElement where the frozen EMPTY_OBJECT could potentially be mutated, causing a TypeError in strict mode.

The Problem

EMPTY_OBJECT is defined as Object.freeze({}) - a frozen, immutable object. In useRenderElementProps, there's a fallback path where outProps could be assigned EMPTY_OBJECT:

const outProps = enabled
  ? (mergeObjects(stateProps, ...) ?? EMPTY_OBJECT)  // <- fallback to frozen object
  : EMPTY_OBJECT;

Later, the code attempts to mutate outProps:

outProps.ref = useMergedRefs(...);      // line 86-88
outProps.className = mergeClassNames(...);  // line 97
outProps.style = mergeObjects(...);         // line 101

If mergeObjects() returns undefined, mutations on the frozen EMPTY_OBJECT would:

  • Fail silently in non-strict mode
  • Throw TypeError in strict mode (which React uses)

Current Status

The bug is latent but not currently triggerable because getStateAttributesProps() always returns a new {}. However, the code is fragile and any refactoring could surface this issue.

The Fix

Replace ?? EMPTY_OBJECT with ?? {} to ensure outProps is always a fresh mutable object when enabled=true.

- ? (mergeObjects(...) ?? EMPTY_OBJECT)
+ ? (mergeObjects(...) ?? {})

Tests Added

  • 4 new tests verifying edge cases with minimal props configuration
  • All 4938 existing JSDOM tests pass
  • All 18 Chromium tests for useRenderElement pass

@Jkker
[utils] Fix potential EMPTY_OBJECT mutation in useRenderElement
69cde48 
Replace EMPTY_OBJECT fallback with a fresh empty object literal to prevent
potential TypeError when setting ref, className, or style properties.
EMPTY_OBJECT is frozen and mutations would fail in strict mode.

Add tests to verify the fix handles edge cases with minimal props.
@pkg-pr-new
Copy link

pkg-pr-new bot commented Feb 5, 2026 •
edited
Loading

commit: d61915f

@netlify
Copy link

netlify bot commented Feb 5, 2026 •
edited
Loading

✅ Deploy Preview for base-ui ready!

Name Link
🔨 Latest commit d61915f
🔍 Latest deploy log https://app.netlify.com/projects/base-ui/deploys/69852c4c1bad30000887c07a
😎 Deploy Preview https://deploy-preview-3986--base-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@mui-bot
Copy link

mui-bot commented Feb 5, 2026 •
edited
Loading

Bundle size report

Bundle Parsed size Gzip size
@base-ui/react 0B(0.00%) 🔺+1B(0.00%)

Details of bundle changes

Check out the code infra dashboard for more information about this PR.

@romgrk romgrk added type: bug It doesn't behave as expected. internal Behind-the-scenes enhancement. Formerly called “core”. labels Feb 5, 2026
@romgrk
Copy link
Contributor

romgrk commented Feb 5, 2026

@Jkker Is there a reason to keep the PR as a draft, or is it ready for merging?

@romgrk
Copy link
Contributor

romgrk commented Feb 6, 2026

That variable could be renamed to mutableProps to make its requirement more explicit.

@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 6, 2026 •
edited
Loading

This PR fixes a latent bug in useRenderElement where the frozen EMPTY_OBJECT could potentially be mutated, causing a TypeError in strict mode.

Could we update the tests so they fail without the change to the source?

LukasTy
LukasTy reviewed Feb 12, 2026
View reviewed changes
packages/react/src/utils/useRenderElement.test.tsx
it('renders correctly with className when using minimal props', async () => {
const { container } = await render(<MinimalComponent className="custom-class" />);
const element = container.firstElementChild;
expect(element).to.have.attribute('class', 'custom-class');
Copy link
Member

@LukasTy LukasTy Feb 12, 2026

Choose a reason for hiding this comment

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

Looks like we could add this assertion after L282 (first test) and remove this almost duplicate test.

packages/react/src/utils/useRenderElement.test.tsx
it('renders correctly with style when using minimal props', async () => {
const { container } = await render(<MinimalComponent style={{ color: 'rgb(255, 0, 0)' }} />);
const element = container.firstElementChild as HTMLElement;
expect(element.style.color).to.equal('rgb(255, 0, 0)');
Copy link
Member

@LukasTy LukasTy Feb 12, 2026

Choose a reason for hiding this comment

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

The same comment applies here. Seems like this can be combined with the 2nd test.

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

Reviewers

@LukasTy LukasTy LukasTy left review comments

@romgrk romgrk romgrk approved these changes

@atomiks atomiks Awaiting requested review from atomiks atomiks will be requested when the pull request is marked ready for review atomiks is a code owner

@colmtuite colmtuite Awaiting requested review from colmtuite colmtuite will be requested when the pull request is marked ready for review colmtuite is a code owner

@michaldudak michaldudak Awaiting requested review from michaldudak michaldudak will be requested when the pull request is marked ready for review michaldudak is a code owner

Assignees

No one assigned

Labels

internal Behind-the-scenes enhancement. Formerly called “core”. type: bug It doesn't behave as expected.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Jkker @mui-bot @romgrk @oliviertassinari @LukasTy

Comments