fix(browser-extension): guard project name against null textContent - #1562
Open
addyCooks wants to merge 1 commit into
Open
fix(browser-extension): guard project name against null textContent#1562addyCooks wants to merge 1 commit into
addyCooks wants to merge 1 commit into
Conversation
createProjectSelectionModal declares its onImport callback with a non-nullable name, but the call site passed Node.textContent, which is typed string | null. Under the tsconfig WXT generates (strict: true), that failed check-types with TS2345 on the pinned TypeScript 5.8.3. Fall back to an empty string, matching the containerTag line directly below it and every other textContent read in the extension. textContent is never null for these option elements, so runtime behaviour is unchanged.
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.
Fixes #1561
Problem
createProjectSelectionModaldeclares itsonImportcallback with anon-nullable
name(utils/ui-components.ts:405-411), but the call site passedNode.textContent, which is typedstring | null. WXT's generated tsconfigsets
strict: true, socheck-typesfailed on the pinned TypeScript 5.8.3:utils/ui-components.ts(611,13): error TS2345:
Argument of type '{ id: string; name: string | null; containerTag: string; }'
is not assignable to parameter of type '{ id: string; name: string; containerTag: string; }'.
CONTRIBUTING.md instructs contributors to run
bun run check-types, so anyonefollowing the documented workflow hit this.
Fix
Fall back to an empty string. This matches the
containerTagline directlybelow it and every other
.textContentread in the extension all 34 of themalready guard with
|| ""or?.; this was the only unguarded site.selectedOption.textwas considered instead, since it is typed non-nullable,but rejected: per spec it strips and collapses whitespace, which would silently
trim project names.
Verification
Against the exact lockfile toolchain (
wxt@0.20.18,typescript@5.8.3):main, confirmed it was the only error inthe package, and confirmed
bun run check-typesexits 0 after the fix<option>elements:
textContentis never null for these nodes, the old and newexpressions are identical across empty, whitespace-only, padded, emoji,
newline and 5000-character names, and whitespace is preserved
selectedIndexis never -1 and theif (selectedOption.value)check short-circuits beforenameis readRuntime behaviour is unchanged this is a type-safety fix.