Export getResponsiveAttributes from experimental entrypoint#8144
Export getResponsiveAttributes from experimental entrypoint#8144rickyzhangca wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: 7bf9678 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
There was a problem hiding this comment.
Pull request overview
Exports the getResponsiveAttributes helper via the @primer/react/experimental entrypoint so consumers have a supported migration path off the deprecated useResponsiveValue hook.
Changes:
- Re-export
getResponsiveAttributesfrompackages/react/src/experimental/index.ts. - Update the experimental exports snapshot to include
getResponsiveAttributes. - Add a changeset for a minor release of
@primer/react.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/experimental/index.ts | Adds getResponsiveAttributes to the experimental entrypoint exports. |
| packages/react/src/tests/snapshots/exports.test.ts.snap | Updates the snapshot to reflect the new experimental export. |
| .changeset/export-get-responsive-attributes.md | Adds a changeset documenting the new export. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Low
Promotes the getResponsiveAttributes helper from an internal util to the public @primer/react/experimental entrypoint (placed next to Hidden). The source file is moved out of src/internal/utils into src/utils to match the precedent set when useMergedRefs was made public, rather than re-exporting from internal. It is the helper referenced by the useResponsiveValue deprecation notice and ADR-018 for bridging responsive prop values to CSS via data-* attributes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
9ec4784 to
6a448a9
Compare
joshblack
left a comment
There was a problem hiding this comment.
Is this something broadly applicable / helping with migration downstream? The ADR itself is definitely written with primer/react in mind so that's why this stuff is more internal / hasn't been exported before as it was just our convention for doing this thing.
Are we seeing downstream that this would be helpful to adopt for useResponsiveValue migration? 👀
This export is referenced in the `useResponsiveValue` deprecation notice and ADR-018.
yes we will use it to replace we will migrate them in 4 ways based on the situation
e.g. before after |
|
@rickyzhangca for that example, I feel like a more tailored migration would be to either use media queries or if they're using tailwind it would be to use the breakpoint prefix for the class name they want to apply, right? e.g. |
|
Parking this as a draft. Let's prefer |
curious, what's our stand on tailwind? is it for team to opt-in while we default to css modules? |
|
@rickyzhangca I think you're spot on, the default is still CSS Modules but tailwind is available to use in github-ui 👍 |
joshblack
left a comment
There was a problem hiding this comment.
Just wanted to approve in advance if we end up finding use-cases 👍
Exports the
getResponsiveAttributeshelper from the@primer/react/experimentalentrypoint. It was previously only available as an internal util, even though it is the helper referenced by theuseResponsiveValue@deprecatednotice and ADR-018 as the sanctioned way to bridge responsive prop values to CSS viadata-*attributes.Consumers migrating off the deprecated
useResponsiveValuehook currently have no supported way to import this helper. Placing it inexperimentalmatches where the sibling responsive primitiveHiddenalready lives.To keep the source location consistent with its new public status, the helper is moved out of
src/internal/utilsintosrc/utils(and its 4 internal importers —PageHeader,PageLayout,SegmentedControl,Stack— are repointed), matching the precedent set whenuseMergedRefswas promoted from internal to public, rather than re-exporting aninternal/path from a public entrypoint.Changelog
New
getResponsiveAttributesfrom@primer/react/experimental.Changed
getResponsiveAttributesfromsrc/internal/utilstosrc/utils(no behavior change; internal import paths updated).Removed
Rollout strategy
Testing & Reviewing
Additive and low-risk. The helper's implementation is unchanged (its unit tests moved alongside it and still pass). The
experimentalexports snapshot was updated to include the new symbol, and aminorchangeset was added. Scopedexports.test.tspasses (3/3), the moved/changed files lint clean, and type-check reports nosrc/errors.