Fix TypeScript strict null check error in create_issue.cjs#15721
Conversation
Wrap temporaryId with String() before passing to normalizeTemporaryId(). TypeScript's strict null checking was complaining that temporaryId could be null even though we checked for error earlier. The String() wrapper satisfies TypeScript's type system while maintaining the same runtime behavior (normalizeTemporaryId already does String() conversion internally). Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
d0fdfb0
into
copilot/fix-ci-pipeline-issue
There was a problem hiding this comment.
Pull request overview
This PR fixes a TypeScript strict null check error in create_issue.cjs by wrapping the temporaryId variable with String() before passing it to normalizeTemporaryId(). While TypeScript's control flow analysis should recognize that temporaryId is non-null after the early return check and JSDoc type cast, this fix ensures the code passes strict null checking.
Changes:
- Added
String()wrapper aroundtemporaryIdwhen callingnormalizeTemporaryId()to satisfy TypeScript's strict null checking - Extracted normalization to intermediate variable for clarity
- Added clarifying comment explaining the null-safety guarantee
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Store the mapping of temporary_id -> {repo, number} | ||
| temporaryIdMap.set(normalizeTemporaryId(temporaryId), { repo: qualifiedItemRepo, number: issue.number }); | ||
| // temporaryId is guaranteed to be non-null because we checked tempIdResult.error above | ||
| const normalizedTempId = normalizeTemporaryId(String(temporaryId)); |
There was a problem hiding this comment.
The String() wrapper is redundant because normalizeTemporaryId() already calls String() internally on its parameter (see temporary_id.cjs line 53). This means the string conversion happens twice.
While this doesn't change runtime behavior, it adds unnecessary code. Since the temporaryId variable at line 321 already has a JSDoc type cast to string, and there's an early return if tempIdResult.error is truthy (lines 313-318), the TypeScript type checker should recognize that temporaryId is non-null at this point.
If TypeScript's control flow analysis isn't recognizing the JSDoc cast, a more idiomatic solution would be to add a runtime check (e.g., if (!temporaryId) throw new Error(...)) or use a non-null assertion if you're confident about the guarantee. The redundant String() call works but masks the underlying type narrowing issue.
| const normalizedTempId = normalizeTemporaryId(String(temporaryId)); | |
| const normalizedTempId = normalizeTemporaryId(temporaryId); |
* Initial plan * Changes before error encountered Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Refactor temporary ID validation into reusable helper function Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Fix TypeScript strict null check error in create_issue.cjs (#15721) * Initial plan * Fix TypeScript null check error in create_issue.cjs line 488 Wrap temporaryId with String() before passing to normalizeTemporaryId(). TypeScript's strict null checking was complaining that temporaryId could be null even though we checked for error earlier. The String() wrapper satisfies TypeScript's type system while maintaining the same runtime behavior (normalizeTemporaryId already does String() conversion internally). Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Fix failing GitHub Actions workflow for Integration: Workflow Misc Part 2 (#15722) * Initial plan * Fix docker_api_proxy_test expectations for LLM gateway support Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Co-authored-by: Peli de Halleux <pelikhan@users.noreply.github.com>
TypeScript type check was failing in the
jsworkflow at line 488 ofcreate_issue.cjs. The functionnormalizeTemporaryId()expects astringparameter, but receivedtemporaryIdwith typestring | nullfromgetOrGenerateTemporaryId().Changes
temporaryIdwithString()before passing tonormalizeTemporaryId()While the JSDoc type cast
/** @type {string} */and early return on error guaranteetemporaryIdis non-null at runtime, TypeScript's strict null checking (strictNullChecks: true) requires explicit type narrowing. TheString()wrapper satisfies this without changing runtime behavior sincenormalizeTemporaryId()already performs string coercion internally.Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
https://api.github.com/repos/github/gh-aw/contents/.github%2Fworkflows%2Faudit-workflows.md/opt/hostedtoolcache/node/24.13.0/x64/bin/node /opt/hostedtoolcache/node/24.13.0/x64/bin/node --conditions node --conditions development --experimental-import-meta-resolve --require /home/REDACTED/work/gh-aw/gh-aw/actions/setup/js/node_modules/vitest/suppress-warnings.cjs /home/REDACTED/work/gh-aw/gh-aw/actions/setup/js/node_modules/vitest/dist/workers/forks.js(http block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.