expo-refresh

[Makisuo]

Greptile Summary

This PR adds a refreshConfiguration native function (iOS + Android) and wires it up in SuperwallProvider so that, in development mode, the Superwall SDK configuration is automatically refreshed after Metro/Fast Refresh cycles — avoiding the need for a full app restart to pick up dashboard config updates.

Confidence Score: 5/5

Safe to merge — all findings are P2 style suggestions with no correctness impact on the Metro-refresh path.

The core feature is logically correct and well-guarded by the DEV flag and the initial-observe skip. All remaining comments are P2: the iOS promise never-reject is benign for the current fire-and-forget caller, and the stale-ref edge case requires two separate non-batched state updates in a Metro refresh scenario (unlikely in practice). The test suite is thorough and covers the key behavioural contracts.

src/SuperwallProvider.tsx (stale-ref guard), ios/SuperwallExpoModule.swift (missing reject path)

Important Files Changed

Filename	Overview
src/SuperwallProvider.tsx	Adds Metro-refresh detection via a hasObservedConfiguredRef / refreshStateRef snapshot pair; reads isLoading and configurationError from a render-snapshot ref inside an [isConfigured]-only effect, which can silently skip a refresh if those fields are updated in a later render.
ios/SuperwallExpoModule.swift	Adds refreshConfiguration async function; resolves promise inside the completion closure but has no error path — the promise always resolves nil regardless of whether the SDK encountered an error.
android/src/main/java/expo/modules/superwallexpo/SuperwallExpoModule.kt	Adds refreshConfiguration async function that calls the Android SDK synchronously and resolves the promise immediately, with proper error rejection on Throwable.
src/SuperwallExpoModule.ts	Adds refreshConfiguration(): Promise<void> to the native module declaration; minor reformatting of didHandleCustomCallback signature.
src/tests/sdk.behavior.test.tsx	Good coverage of the new behaviour: initial-configure skip, dev-rerun trigger, production guard, error catch — all with proper mocking of refreshConfiguration.

Sequence Diagram

sequenceDiagram
    participant Metro as Metro (Fast Refresh)
    participant SW as SuperwallProvider
    participant Ref as refreshStateRef / hasObservedConfiguredRef
    participant Native as SuperwallExpoModule (native)
    participant SDK as SuperwallKit / Android SDK

    Note over SW: App starts — initial configure()
    SW->>Ref: hasObservedConfiguredRef = false
    SDK-->>SW: isConfigured → true
    SW->>Ref: hasObservedConfiguredRef = true (skip refresh)

    Note over Metro: Developer triggers Fast Refresh
    Metro->>SDK: SDK re-initialises
    SDK-->>SW: isConfigured → false
    SDK-->>SW: isConfigured → true
    SW->>Ref: Read refreshStateRef (isLoading=false, configError=null)
    SW->>Ref: hasObservedConfiguredRef already true → proceed
    SW->>Native: refreshConfiguration()
    Native->>SDK: Superwall.shared.refreshConfiguration (iOS callback) / Superwall.instance.refreshConfiguration() (Android sync)
    SDK-->>Native: done
    Native-->>SW: Promise resolves

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: ios/SuperwallExpoModule.swift
Line: 138-145

Comment:
**Promise never rejects on iOS**

The completion closure only ever calls `promise.resolve(nil)`, so any internal failure in `Superwall.shared.refreshConfiguration` is silently swallowed on iOS. The Android counterpart wraps the call in `try/catch` and calls `promise.reject(CodedException(error))` on failure. While the TypeScript caller currently uses `.catch()` for Metro-refresh, future callers awaiting `refreshConfiguration()` would receive a resolved promise even when the SDK failed on iOS. Consider adding error propagation — if the SuperwallKit closure provides an `Error?` parameter, reject there; otherwise wrap the call in a `do/catch`.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/SuperwallProvider.tsx
Line: 137-152

Comment:
**Refresh can be silently skipped when `isLoading` lags behind `isConfigured`**

`isLoading` and `configurationError` are read from `refreshStateRef.current` inside an effect that only depends on `[isConfigured]`. If the store emits `isConfigured = true` in one render and then clears `isLoading` in a subsequent render (i.e. the two state updates are not batched), the effect runs while `isLoading` is still `true` and exits early — the refresh is never attempted, with no log or retry. This is an edge case, but Metro-refresh timing makes it more likely than typical usage.

A safer alternative is to also depend on `isLoading` and `configurationError` directly:

```ts
useEffect(() => {
  if (!__DEV__ || !isConfigured || isLoading || configurationError) {
    return
  }

  if (!hasObservedConfiguredRef.current) {
    hasObservedConfiguredRef.current = true
    return
  }

  SuperwallExpoModule.refreshConfiguration().catch((error) => {
    console.error("[Superwall] Failed to refresh configuration after Metro refresh", error)
  })
}, [isConfigured, isLoading, configurationError])
```

The `hasObservedConfiguredRef` already prevents spurious calls, so the extra dependencies don't add refresh invocations — they only ensure the effect re-evaluates when `isLoading` or the error clears after `isConfigured` is set.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "expo-refresh" | Re-trigger Greptile}

Greptile also left 2 inline comments on this PR.

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/expo-superwall@189

commit: 6e9a913

[greptile-apps]

Promise never rejects on iOS

The completion closure only ever calls promise.resolve(nil), so any internal failure in Superwall.shared.refreshConfiguration is silently swallowed on iOS. The Android counterpart wraps the call in try/catch and calls promise.reject(CodedException(error)) on failure. While the TypeScript caller currently uses .catch() for Metro-refresh, future callers awaiting refreshConfiguration() would receive a resolved promise even when the SDK failed on iOS. Consider adding error propagation — if the SuperwallKit closure provides an Error? parameter, reject there; otherwise wrap the call in a do/catch.

Prompt To Fix With AI

This is a comment left during a code review.
Path: ios/SuperwallExpoModule.swift
Line: 138-145

Comment:
**Promise never rejects on iOS**

The completion closure only ever calls `promise.resolve(nil)`, so any internal failure in `Superwall.shared.refreshConfiguration` is silently swallowed on iOS. The Android counterpart wraps the call in `try/catch` and calls `promise.reject(CodedException(error))` on failure. While the TypeScript caller currently uses `.catch()` for Metro-refresh, future callers awaiting `refreshConfiguration()` would receive a resolved promise even when the SDK failed on iOS. Consider adding error propagation — if the SuperwallKit closure provides an `Error?` parameter, reject there; otherwise wrap the call in a `do/catch`.

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

Refresh can be silently skipped when isLoading lags behind isConfigured

isLoading and configurationError are read from refreshStateRef.current inside an effect that only depends on [isConfigured]. If the store emits isConfigured = true in one render and then clears isLoading in a subsequent render (i.e. the two state updates are not batched), the effect runs while isLoading is still true and exits early — the refresh is never attempted, with no log or retry. This is an edge case, but Metro-refresh timing makes it more likely than typical usage.

A safer alternative is to also depend on isLoading and configurationError directly:

useEffect(() => {
  if (!__DEV__ || !isConfigured || isLoading || configurationError) {
    return
  }

  if (!hasObservedConfiguredRef.current) {
    hasObservedConfiguredRef.current = true
    return
  }

  SuperwallExpoModule.refreshConfiguration().catch((error) => {
    console.error("[Superwall] Failed to refresh configuration after Metro refresh", error)
  })
}, [isConfigured, isLoading, configurationError])

The hasObservedConfiguredRef already prevents spurious calls, so the extra dependencies don't add refresh invocations — they only ensure the effect re-evaluates when isLoading or the error clears after isConfigured is set.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/SuperwallProvider.tsx
Line: 137-152

Comment:
**Refresh can be silently skipped when `isLoading` lags behind `isConfigured`**

`isLoading` and `configurationError` are read from `refreshStateRef.current` inside an effect that only depends on `[isConfigured]`. If the store emits `isConfigured = true` in one render and then clears `isLoading` in a subsequent render (i.e. the two state updates are not batched), the effect runs while `isLoading` is still `true` and exits early — the refresh is never attempted, with no log or retry. This is an edge case, but Metro-refresh timing makes it more likely than typical usage.

A safer alternative is to also depend on `isLoading` and `configurationError` directly:

```ts
useEffect(() => {
  if (!__DEV__ || !isConfigured || isLoading || configurationError) {
    return
  }

  if (!hasObservedConfiguredRef.current) {
    hasObservedConfiguredRef.current = true
    return
  }

  SuperwallExpoModule.refreshConfiguration().catch((error) => {
    console.error("[Superwall] Failed to refresh configuration after Metro refresh", error)
  })
}, [isConfigured, isLoading, configurationError])
```

The `hasObservedConfiguredRef` already prevents spurious calls, so the extra dependencies don't add refresh invocations — they only ensure the effect re-evaluates when `isLoading` or the error clears after `isConfigured` is set.

How can I resolve this? If you propose a fix, please make it concise.