Route utility exports through non-component modules#7941
Open
joshblack wants to merge 2 commits into
Open
Conversation
|
github-actions
Bot
added
staff
Contributor
|
joshblack
changed the title
joshblack
added
skip changeset
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request refactors utility exports/imports to route them through non-component modules (for example constants.ts/utils.ts) and replaces wildcard/component-file re-exports that interfere with React Refresh enforcement.
Changes:
- Added a
KeybindingHintutility module for generating accessible keybinding strings and adjusted exports to avoid utility re-exports from component files. - Updated several components (Overlay, Token, Timeline, UnderlineNav, SelectPanel2, AnchoredOverlay) and tests/stories to import utilities from
constants.ts/utils.tsinstead of component implementation modules. - Introduced a shared Storybook utility module (
utils/story-helpers.ts) and updated some stories to use explicit exports/imports.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/utils/story-helpers.ts | New Storybook helper utilities (argTypes builders, form control args, Octicon mapping). |
| packages/react/src/UnderlineNav/UnderlineNav.tsx | Removed utility re-export from component implementation file. |
| packages/react/src/Token/TokenBase.tsx | Stopped re-exporting Token utilities/constants from the component module; imports now come from constants.ts/utils.ts. |
| packages/react/src/Token/Token.tsx | Updated imports to use constants.ts/utils.ts instead of TokenBase re-exports. |
| packages/react/src/Token/IssueLabelToken.tsx | Updated imports to use constants.ts/utils.ts instead of TokenBase re-exports. |
| packages/react/src/Token/_RemoveTokenButton.tsx | Updated defaultTokenSize import to come from constants.ts. |
| packages/react/src/Token/tests/Token.test.tsx | Updated tokenSizes import to come from constants.ts. |
| packages/react/src/Timeline/Timeline.tsx | Removed utility re-export from component implementation file and adjusted how constants are referenced. |
| packages/react/src/Timeline/Timeline.stories.tsx | Updated story imports to pull badge variants from constants.ts. |
| packages/react/src/TextInputWithTokens/TextInputWithTokens.test.tsx | Updated tokenSizes import to come from Token constants.ts. |
| packages/react/src/Overlay/Overlay.tsx | Removed constants re-export from component implementation file and adjusted import style. |
| packages/react/src/KeybindingHint/utils.ts | New utility module for accessible keybinding hint strings. |
| packages/react/src/KeybindingHint/KeybindingHint.tsx | Moved plain-string utility out of the component module. |
| packages/react/src/KeybindingHint/index.ts | Switched from wildcard export to explicit exports, routing the utility through utils.ts. |
| packages/react/src/KeybindingHint/components/Sequence.tsx | Removed re-export of internal accessibility utility. |
| packages/react/src/KeybindingHint/components/Chord.tsx | Removed re-export of internal accessibility utility. |
| packages/react/src/experimental/SelectPanel2/SelectPanel.tsx | Updated heightMap import to come from Overlay/constants.ts. |
| packages/react/src/experimental/SelectPanel2/index.tsx | Replaced wildcard export with explicit value/type exports. |
| packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx | Updated widthMap import to come from Overlay/constants.ts. |
Copilot's findings
Comments suppressed due to low confidence (1)
packages/react/src/Overlay/Overlay.tsx:17
heightMap/widthMapare imported usingimport type, but the prop types reference them viakeyof typeof ….typeofin a type query requires a value symbol, so this combination will fail type-checking.
Use the imported types directly (drop the typeof), or switch back to a value import.
import type {heightMap, widthMap} from './constants'
type StyledOverlayProps = {
width?: keyof typeof widthMap
height?: keyof typeof heightMap
- Files reviewed: 18/19 changed files
- Comments generated: 1
|
|
||
| export {TimelineBadgeVariants} | ||
|
|
||
| export type TimelineBadgeVariant = (typeof TimelineBadgeVariants)[number] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #
No linked issue.
Routes utility exports through non-component modules and replaces wildcard/component-file re-exports that block React Refresh enforcement.
Changelog
New
Changed
.tsxto.tsbecause they do not export components.Removed
Rollout strategy
Testing & Reviewing
Focus review on ensuring utility imports now point at
constants.ts,utils.ts, or package barrels instead of component files.Merge checklist