-
Notifications
You must be signed in to change notification settings - Fork 15.7k
fix: 修复条件式 hook 调用导致的 "Rendered fewer hooks than expected" 错误 #435
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -25,24 +25,24 @@ export function useFrustrationDetection( | |||||
| const [state, setState] = useState<FrustrationState>('closed') | ||||||
|
|
||||||
| const config = getGlobalConfig() as { transcriptShareDismissed?: boolean } | ||||||
| if (config.transcriptShareDismissed) { | ||||||
| return { state: 'closed', handleTranscriptSelect: () => {} } | ||||||
| } | ||||||
|
|
||||||
| if (!isPolicyAllowed('product_feedback' as any)) { | ||||||
| return { state: 'closed', handleTranscriptSelect: () => {} } | ||||||
| } | ||||||
|
|
||||||
| if (isLoading || hasActivePrompt || otherSurveyOpen) { | ||||||
| return { state: 'closed', handleTranscriptSelect: () => {} } | ||||||
| } | ||||||
| const policyAllowed = isPolicyAllowed('product_feedback' as any) | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
♻️ Proposed fix (if `PolicyKey` is the union type)- const policyAllowed = isPolicyAllowed('product_feedback' as any)
+ const policyAllowed = isPolicyAllowed('product_feedback' as unknown as PolicyKey)Alternatively, add As per coding guidelines: "Prohibit 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| const shouldSkip = | ||||||
| config.transcriptShareDismissed || | ||||||
| !policyAllowed || | ||||||
| isLoading || | ||||||
| hasActivePrompt || | ||||||
| otherSurveyOpen | ||||||
|
|
||||||
| const frustrated = detectFrustration(messages) | ||||||
|
|
||||||
| const effectiveState = | ||||||
| frustrated && state === 'closed' ? 'transcript_prompt' : state | ||||||
| const effectiveState = shouldSkip | ||||||
| ? 'closed' | ||||||
| : frustrated && state === 'closed' | ||||||
| ? 'transcript_prompt' | ||||||
| : state | ||||||
|
|
||||||
| function handleTranscriptSelect(choice: string) { | ||||||
| const handleTranscriptSelect = (choice: string) => { | ||||||
| if (shouldSkip) return | ||||||
| if (choice === 'yes') { | ||||||
| void submitTranscriptShare(messages, 'frustration', crypto.randomUUID()) | ||||||
| setState('submitted') | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,4 +1,4 @@ | ||||||||||||||||||
| import { useState } from 'react' | ||||||||||||||||||
| import { useRef } from 'react' | ||||||||||||||||||
| import { major, minor, patch } from 'semver' | ||||||||||||||||||
|
|
||||||||||||||||||
| export function getSemverPart(version: string): string { | ||||||||||||||||||
|
|
@@ -17,18 +17,17 @@ export function useUpdateNotification( | |||||||||||||||||
| updatedVersion: string | null | undefined, | ||||||||||||||||||
| initialVersion: string = MACRO.VERSION, | ||||||||||||||||||
| ): string | null { | ||||||||||||||||||
| const [lastNotifiedSemver, setLastNotifiedSemver] = useState<string | null>( | ||||||||||||||||||
| () => getSemverPart(initialVersion), | ||||||||||||||||||
| ) | ||||||||||||||||||
| const lastNotifiedRef = useRef<string | null>(getSemverPart(initialVersion)) | ||||||||||||||||||
|
|
||||||||||||||||||
| if (!updatedVersion) { | ||||||||||||||||||
| const updatedSemver = updatedVersion ? getSemverPart(updatedVersion) : null | ||||||||||||||||||
| if (!updatedSemver) { | ||||||||||||||||||
|
Comment on lines
+22
to
+23
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result: In semver v7 (npm/node-semver package version 7.x), the major, minor, and patch functions throw a TypeError with message "Invalid Version: {version}" when passed an invalid version string. They do not return null.[1][2][3] Citations:
🏁 Script executed: cat -n src/hooks/useUpdateNotification.tsRepository: claude-code-best/claude-code Length of output: 1270 🌐 Web query:
💡 Result: In npm's node-semver library version 7.x (latest 7.7.4 as of 2026), the 'loose' option makes parsing more forgiving for not-quite-valid semver strings, primarily by allowing prerelease tags without the required '-' separator (e.g., semver.valid('1.2.3beta', {loose: true}) returns '1.2.3-beta' [1][2][3]). Output is always strict-compliant semver. For major, minor, patch extraction: Functions like major(v), minor(v), patch(v) first parse/validate v using loose rules if specified, then extract the respective component from the parsed SemVer object [4][2]. If invalid even loosely, they return undefined/null. Invalid versions: semver.valid('a.b.c') returns null even with loose [4]. Loose does not accept non-numeric components in major/minor/patch or malformed structures like missing parts ('2.0' likely null in loose [5]). It may misparse extras like '1.2.34.5' as '1.2.3-4.5' by treating extra as prerelease [5]. No major changes to loose behavior in v7; recent fixes ensure CLI passes loose to valid [1][6]. Use coerce for aggressive non-semver to semver conversion (e.g., '42.6.7.9.3-alpha' -> '42.6.7'), independent of loose [4][7]. Citations:
🌐 Web query:
💡 Result: In the npm/node-semver library, semver.major(v) returns a major number (it’s not documented to return null for invalid versions), and loose:true is only relevant when parsing/validating options are actually used by the underlying logic. The public semver docs state that valid(v) returns null if it’s not valid, while major(v) “Return the major version number” (no null/loose behavior described for major specifically) [1]. So the behavior “semver.major with { loose:true } throws or returns null for an invalid version” is not a specific/known “bug” in the major API itself. What can happen is: 1) If the input string is not valid semver, the code path may throw a TypeError “Invalid Version: …” (common across semver operations when invalid inputs are passed to logic that expects a valid SemVer object) [2][3]. 2) If you use the CLI or other helper code that calls semver.valid(v, options), loose:true can change validation results (for example, the npm CLI had a loose-flag validation issue because it called semver.valid(v) without passing the loose option, and it was fixed to call semver.valid(v, options)) [4]. Practical takeaway: don’t rely on semver.major(v, { loose:true }) returning null for invalid input. If you need “null instead of throw,” call semver.valid(v, { loose: true }) first; docs explicitly say valid(v) “Return the parsed version, or null if it’s not valid” [1]. Minimal example pattern (node-semver): - if (!semver.valid(input, { loose: true })) return null; - else return semver.major(input); Citations: semver.valid returns parsed version or null [1]; semver.major is described as returning the major version number [1]; npm CLI loose validation bug/fix indicates passing loose into semver.valid is what changes loose behavior [4]; invalid version commonly throws TypeError in semver operations [2][3]. Citations:
Guard semver parsing before calling A truthy but invalid Proposed fix-import { major, minor, patch } from 'semver'
+import { major, minor, patch, valid } from 'semver'
@@
- const updatedSemver = updatedVersion ? getSemverPart(updatedVersion) : null
+ const normalizedUpdatedVersion =
+ updatedVersion && valid(updatedVersion, { loose: true })
+ ? updatedVersion
+ : null
+ const updatedSemver = normalizedUpdatedVersion ? getSemverPart(normalizedUpdatedVersion) : null📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
| return null | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| const updatedSemver = getSemverPart(updatedVersion) | ||||||||||||||||||
| if (updatedSemver !== lastNotifiedSemver) { | ||||||||||||||||||
| setLastNotifiedSemver(updatedSemver) | ||||||||||||||||||
| if (updatedSemver !== lastNotifiedRef.current) { | ||||||||||||||||||
| lastNotifiedRef.current = updatedSemver | ||||||||||||||||||
| return updatedSemver | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| return null | ||||||||||||||||||
| } | ||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update stale comment to match
NODE_ENV="development"behavior.Line 19 still says “React production mode”, but Line 21 now sets development. Please align the comment to avoid misleading future changes.
🤖 Prompt for AI Agents