Skip to content

refactor: silence optString(name, null) Kotlin warnings#144

Merged
riteshshukla04 merged 2 commits into
margelo:mainfrom
uwayss:refactor/optstring-kotlin-warnings
Jul 17, 2026
Merged

refactor: silence optString(name, null) Kotlin warnings#144
riteshshukla04 merged 2 commits into
margelo:mainfrom
uwayss:refactor/optstring-kotlin-warnings

Conversation

@uwayss

@uwayss uwayss commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Building the Android modules gives 23 compiler warnings, all the same one:

w: Java type mismatch: inferred type is 'Nothing?', but 'String' was expected

They come from optString(name, null)optString's fallback param is @NonNull, so passing null warns. 13 in AutoPrefetcher.kt, 10 in NitroWebSocketAutoPrewarmer.kt.

This adds an optStringOrNull() helper to each android package and moves all 23 calls onto it. compileReleaseKotlin on both modules: 23 warnings → 0.

internal fun JSONObject.optStringOrNull(name: String): String? =
  if (isNull(name)) null else optString(name)

isNull() maps both a missing key and an explicit "key": null to null.

A couple of small things:

  • NitroWebSocketAutoPrewarmer.kt:206 keeps optString(j, null) — that's the JSONArray overload, no @NonNull, never warned.
  • Helper is internal (kept out of the published AAR) and duplicated per package, since the websockets module doesn't depend on the fetch one — same as NitroLogger.kt.

Test plan

  • compileReleaseKotlin both modules — 23 → 0
  • bun lint, bun typecheck, bun test — all pass

🤖 Generated with Claude Code

JSONObject.optString's `fallback` parameter is annotated @nonnull, so
passing null produced 23 "Java type mismatch: inferred type is
'Nothing?', but 'String' was expected" warnings across AutoPrefetcher
and NitroWebSocketAutoPrewarmer.

Add an internal optStringOrNull() extension to each package and use it
at all 23 call sites.

Behaviour is unchanged. The helper guards on has() rather than the more
obvious isNull() because optString coerces the JSONObject.NULL sentinel
to the string "null" -- so an explicit `"key": null` returns "null" and
only an absent key returns null. Using isNull() would have been a
silent behaviour change rather than a warning fix.

The helper is duplicated rather than shared because the websockets
android module depends only on :react-native-nitro-modules, not on the
fetch package. This matches the existing NitroLogger.kt precedent.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 16, 2026

Copy link
Copy Markdown

@uwayss is attempting to deploy a commit to the Margelo Team on Vercel.

A member of the Team first needs to authorize it.

@riteshshukla04 riteshshukla04 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for PR. This is good stuff. I just have 2 issues here

  1. Lots of AI slop comments (Max 1 line comment)
  2. The check has an issue . Something like {"foo": null} would return empty string in optString

Guard on isNull() instead of has() so a `"key": null` value returns null
rather than a coerced string, and trim the helper comment to one line.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@uwayss

uwayss commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

Both fixed in d4e568e — comments down to one line, and switched to isNull() so {"foo": null} returns null.

@riteshshukla04
riteshshukla04 merged commit 5c1c9a8 into margelo:main Jul 17, 2026
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants