Fix TypeError in LanguageDropdown when used in multiple mode#5823
Fix TypeError in LanguageDropdown when used in multiple mode#5823akolson wants to merge 1 commit intolearningequality:hotfixesfrom
Conversation
VAutocomplete eagerly evaluates getText(internalValue) as the fallback argument to getValue, even when the fallback is never used. In multiple mode (used in SearchFilters), internalValue is an Array of selected ids. Arrays are objects in JS, so getPropertyFromItem does not short-circuit on the primitive guard and calls languageText with the array directly. Since arrays have no native_name property, this throws a TypeError. Guard languageText against item being null/undefined or lacking a native_name (covers arrays, partial objects, and any future edge cases). Fixes learningequality#5740. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rtibblesbot
left a comment
There was a problem hiding this comment.
Tight, well-reasoned hotfix for the Sentry crash — guard and tests are exactly right.
CI passing. Phase 3: no dev server available; the template is entirely unchanged (only the languageText method was modified), so there is nothing visual to verify — the fix's correctness is established by the new unit tests and CI.
- praise: see inline (×2)
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Reviewed the pull request diff checking for:
- Correctness: bugs, edge cases, undocumented behavior, resource leaks, hardcoded values
- Design: unnecessary complexity, naming, readability, comment accuracy, redundant state
- Architecture: duplicated concerns, minimal interfaces, composition over inheritance
- Testing: behavior-based assertions, mocks only at hard boundaries, accurate coverage
- Completeness: missing dependencies, unupdated usages, i18n, accessibility, security
- Principles: DRY (same reason to change), SRP, Rule of Three (no premature abstraction)
- Checked CI status and linked issue acceptance criteria
- For UI changes: inspected screenshots for layout, visual completeness, and consistency
| }, | ||
| methods: { | ||
| languageText(item) { | ||
| // VAutocomplete eagerly evaluates getText(internalValue) as a fallback arg to |
There was a problem hiding this comment.
praise: The comment explaining Vuetify's eager getText(internalValue) argument evaluation is exactly the right thing to document here — this is non-obvious framework internals and would otherwise pull future maintainers into a VAutocomplete/VSelect source dive to understand why the guard exists.
| // getValue, even when that fallback isn't needed. In multiple mode, internalValue | ||
| // is an Array, and arrays have no native_name. Guard against both that case and | ||
| // any other scenario where item or native_name is missing. | ||
| if (!item || item.native_name == null) { |
There was a problem hiding this comment.
praise: == null (rather than !item.native_name) is the precisely correct check. It catches null, undefined, and array inputs (where .native_name is undefined) while deliberately allowing native_name: '' through so existing behaviour for empty-string entries is preserved. The companion test at line 82 of the spec explicitly pins this distinction — good defensive coding.
Summary
LanguageDropdownis used withmultipleprop inSearchFilters.vue. In that mode,v-modelholds an Array of selected language IDs. VAutocomplete'supdateAutocompletepassesthis.internalValue(the Array) togetValue, which eagerly evaluatesgetText(internalValue)as a JS argument before callinggetPropertyFromItem— even though that fallback is never actually needed.getTextin turn callslanguageText(array). Arrays are objects in JS, so the primitive short-circuit guard ingetPropertyFromItemdoes not apply, andlanguageTextis invoked with the array directly. Since arrays have nonative_name,item.native_name.splitthrows a TypeError.Fix: guard
languageTextagainstitembeing null/undefined or lacking anative_nameproperty (covers arrays, partial objects, and any future edge cases) by returning an empty string early.Verification: ran the existing
languageDropdowntest suite and confirmed all tests pass, including two new cases covering the missing-native_nameand array-input scenarios.References
Fixes #5740
Recurrence of the issue addressed by #5465 (the null check was subsequently reverted without a documented reason).
Reviewer guidance
LanguageDropdown.vue:120— the new guard uses== null(not!item.native_name) deliberately, to avoid catching empty-stringnative_namevalues that the existing test exercises.SearchFilters.vue:68-71— this is wheremultipleis used; reproduce the crash by opening Import from Channels, selecting a language filter, then blurring the dropdown.languageTextin the dropdown component.AI usage
I used Claude Code to trace through the Vuetify VAutocomplete and VSelect source to identify why
languageTextwas receiving an Array argument in multiple mode. After understanding the root cause (eager argument evaluation ingetValue), I wrote the guard and two new test cases. I reviewed the generated code and confirmed the condition== nullis correct for the intended behaviour.