Skip to content

Commit 551ec63

Browse files
authored
perf(Button): fix CounterLabel remount, remove DEV hook, enable React Compiler (#7552)
1 parent f25bb1c commit 551ec63

File tree

3 files changed

+21
-24
lines changed

3 files changed

+21
-24
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@primer/react': patch
3+
---
4+
5+
perf(Button): fix CounterLabel remount and remove conditional DEV hook

packages/react/script/react-compiler.mjs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ const unsupported = new Set(
1515
[
1616
'src/ActionMenu/**/*.tsx',
1717
'src/AvatarStack/**/*.tsx',
18-
'src/Button/**/*.tsx',
1918
'src/ConfirmationDialog/**/*.tsx',
2019
'src/Pagehead/**/*.tsx',
2120
'src/Pagination/**/*.tsx',

packages/react/src/Button/ButtonBase.tsx

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -60,24 +60,19 @@ const ButtonBase = forwardRef(({children, as: Component = 'button', ...props}, f
6060
const ariaDescribedByIds = loading ? [loadingAnnouncementID, ariaDescribedBy] : [ariaDescribedBy]
6161

6262
if (__DEV__) {
63-
/**
64-
* The Linter yells because it thinks this conditionally calls an effect,
65-
* but since this is a compile-time flag and not a runtime conditional
66-
* this is safe, and ensures the entire effect is kept out of prod builds
67-
* shaving precious bytes from the output, and avoiding mounting a noop effect
68-
*/
69-
// eslint-disable-next-line react-hooks/rules-of-hooks
70-
React.useEffect(() => {
71-
if (
72-
innerRef.current &&
73-
!(innerRef.current instanceof HTMLButtonElement) &&
74-
!((innerRef.current as unknown) instanceof HTMLAnchorElement) &&
75-
!((innerRef.current as HTMLElement).tagName === 'SUMMARY')
76-
) {
77-
// eslint-disable-next-line no-console
78-
console.warn('This component should be an instanceof a semantic button or anchor')
79-
}
80-
}, [innerRef])
63+
// Validate that the element is a semantic button/anchor.
64+
// This runs during render (not in an effect) to avoid a conditional hook call
65+
// that prevents React Compiler from optimizing this component.
66+
const el = innerRef.current
67+
if (
68+
el &&
69+
!(el instanceof HTMLButtonElement) &&
70+
!((el as unknown) instanceof HTMLAnchorElement) &&
71+
!((el as HTMLElement).tagName === 'SUMMARY')
72+
) {
73+
// eslint-disable-next-line no-console
74+
console.warn('This component should be an instanceof a semantic button or anchor')
75+
}
8176
}
8277
return (
8378
<ConditionalWrapper
@@ -154,11 +149,9 @@ const ButtonBase = forwardRef(({children, as: Component = 'button', ...props}, f
154149
*/
155150
count !== undefined && !TrailingVisual
156151
? renderModuleVisual(
157-
() => (
158-
<CounterLabel className={classes.CounterLabel} data-component="ButtonCounter">
159-
{count}
160-
</CounterLabel>
161-
),
152+
<CounterLabel className={classes.CounterLabel} data-component="ButtonCounter">
153+
{count}
154+
</CounterLabel>,
162155
Boolean(loading) && !LeadingVisual,
163156
'trailingVisual',
164157
true,

0 commit comments

Comments
 (0)