Skip to content

refactor: remove unnecessary Fragment wrapping#110

Open
TrevorBurnham wants to merge 1 commit into
cloudscape-design:mainfrom
TrevorBurnham:refactor/remove-fragment-wrapping
Open

refactor: remove unnecessary Fragment wrapping#110
TrevorBurnham wants to merge 1 commit into
cloudscape-design:mainfrom
TrevorBurnham:refactor/remove-fragment-wrapping

Conversation

@TrevorBurnham

@TrevorBurnham TrevorBurnham commented Jan 24, 2026

Copy link
Copy Markdown
Contributor

Description

Children.only() already validates that the highlight result is a single element. Wrapping that result in a Fragment first, only to read .props.children back off the wrapper, is a no-op:

-  const codeElementWrapper: ReactElement = createElement(Fragment, null, code);
-  const codeElement = Children.only(codeElementWrapper.props.children);
+  const codeElement = Children.only(code) as ReactElement;

The Children.only() call is kept deliberately. CodeViewProps.highlight is typed (code: string) => React.ReactNode, so a consumer can return a string, an array, or a fragment. Children.only() turns those cases into a clear error rather than silently rendering nothing when the render path reads codeElement.props.children. The old code got its ReactElement typing implicitly from the Fragment wrapper, so the explicit cast replaces that.

No behavior change.

How has this been tested?

Existing unit tests pass (51 tests). Type-checked, linted, and verified in the dev server that the demos render unchanged.

Review checklist

The following items are to be evaluated by the author(s) and the reviewer(s).

Correctness

  • Changes include appropriate documentation updates.
  • Changes are backward-compatible if not indicated, see CONTRIBUTING.md.
  • Changes do not include unsupported browser features, see CONTRIBUTING.md.
  • Changes were manually tested for accessibility, see accessibility guidelines.

Testing

  • Changes are covered with new/existing unit tests?
  • Changes are covered with new/existing integration tests?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@TrevorBurnham
TrevorBurnham requested a review from a team as a code owner January 24, 2026 23:58
@TrevorBurnham
TrevorBurnham requested review from johannes-weber and removed request for a team January 24, 2026 23:58

@pan-kot pan-kot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The changes conflict with main.

Children.only() already validates that the highlight result is a single
element, so wrapping it in a Fragment first just to read .props.children
back off the wrapper is a no-op. Call Children.only() on the result
directly and cast, since the highlight prop is typed as ReactNode.
@TrevorBurnham
TrevorBurnham force-pushed the refactor/remove-fragment-wrapping branch from ae09dde to 7d0cb1a Compare July 26, 2026 19:08
@TrevorBurnham

Copy link
Copy Markdown
Contributor Author

@pan-kot Updated!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants