Add no-use-responsive-value rule #443
Conversation
🦋 Changeset detectedLatest commit: ccb95f5 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
This PR adds a new ESLint rule no-use-responsive-value that prevents the use of the useResponsiveValue hook from @primer/react and local imports. The rule enforces SSR-safe patterns by disallowing this hook which relies on useMediaUnsafeSSR without a default state.
Key changes:
- Implements a new ESLint rule to detect and flag imports of
useResponsiveValue - Adds comprehensive test coverage for various import scenarios
- Includes the rule in the recommended configuration preset
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/rules/no-use-responsive-value.js |
Implements the rule logic to detect useResponsiveValue imports from @primer/react and local paths |
src/rules/__tests__/no-use-responsive-value.test.js |
Provides comprehensive test cases covering valid and invalid import scenarios |
src/index.js |
Exports the new rule from the plugin |
src/configs/recommended.js |
Adds the rule to the recommended configuration as an error |
docs/rules/no-use-responsive-value.md |
Documents the rule's purpose, examples, and usage |
| // Check for import declarations | ||
| ImportDeclaration(node) { | ||
| // Check for @primer/react imports | ||
| const isPrimerImport = /@primer\/react/.test(node.source.value) |
There was a problem hiding this comment.
The regex /@primer\/react/ will match any path containing '@primer/react', including imports like @primer/react-native or some-@primer/react-utils. Use anchored regex /^@primer\/react(\/|$)/ to match only @primer/react and its sub-paths (e.g., /experimental).
| const isPrimerImport = /@primer\/react/.test(node.source.value) | |
| const isPrimerImport = /^@primer\/react(\/|$)/.test(node.source.value) |
| const isPrimerImport = /@primer\/react/.test(node.source.value) | ||
| // Check for local imports that might be useResponsiveValue hook | ||
| const isLocalUseResponsiveValueImport = | ||
| node.source.value.includes('useResponsiveValue') || node.source.value.includes('/hooks/useResponsiveValue') |
There was a problem hiding this comment.
The condition node.source.value.includes('/hooks/useResponsiveValue') is redundant because it's already covered by the first condition node.source.value.includes('useResponsiveValue'). Additionally, this logic will create false positives for paths like '../hooks/useResponsiveValueHelper'. Consider using a more precise check that verifies the path ends with useResponsiveValue (e.g., /useResponsiveValue(\.js)?$/ or checking the last path segment).
| node.source.value.includes('useResponsiveValue') || node.source.value.includes('/hooks/useResponsiveValue') | |
| /(?:^|\/)useResponsiveValue(\.[jt]sx?)?$/.test(node.source.value) |
hectahertz
changed the title
Closes https://github.com/github/primer/issues/5283
Adds a new rule that disallows usage of the
useResponsiveValuehook from@primer/reactor local imports.The rule aims to enforce SSR-safe responsive patterns by preventing usage of a hook known to cause hydration mismatches, and is included in the recommended configuration.
Documentation and tests for the rule are also provided.