Skip to content

fix: replace direct console.* calls with rnWindowLogger in rn-window package#1597

Open
devin-ai-integration[bot] wants to merge 1 commit intomainfrom
devin/1771609729-rn-window-use-sdk-logger
Open

fix: replace direct console.* calls with rnWindowLogger in rn-window package#1597
devin-ai-integration[bot] wants to merge 1 commit intomainfrom
devin/1771609729-rn-window-use-sdk-logger

Conversation

@devin-ai-integration
Copy link
Contributor

Description

Part of the effort to make consoleLogLevel="silent" work across all SDK packages. Replaces 12 direct console.* calls in @crossmint/client-sdk-rn-window with an rnWindowLogger instance (SdkLogger) so they can respect the consoleLogLevel setting.

Files changed:

  • RNWebViewTransport.ts — 8 replacements (info, error, warn calls for message handling, send, reload)
  • Parent.ts — 4 replacements (error, info calls for handshake, recovery)
  • New logger.ts with rnWindowLogger singleton and initRnWindowLogger(apiKey) initializer

Intentionally unchanged:

  • RNWebViewTransport.ts line 54: console.error inside an injected JavaScript string — this runs inside the WebView context where the SDK logger is not available
  • RNWebView.tsx: Console monkey-patches for WebView debugging — these are intentional overrides, not regular log calls

⚠️ Key review points

  1. initRnWindowLogger() is exported but not called anywhere in this diff. Until initialization is wired up (likely in a provider), the logger falls back to raw console.* (SdkLogger uninitialized behavior). This means logs won't actually be silenced yet — same pattern as companion PRs.
  2. New dependency: @crossmint/common-sdk-base added to rn-window for SdkLogger access. Confirm this is acceptable for a React Native package.
  3. detectPlatform() in RN context — the logger init uses browser/server sink selection which may not be the right fit for React Native. Worth verifying detectPlatform() returns something sensible here.

Companion PRs: #1592 (wallets), #1593 (auth), #1594 (window), #1595 (react-base), #1596 (react-ui)

Test plan

  • Lint passes (pnpm lint)
  • Existing unit tests unaffected (no test modifications)
  • Manual verification recommended: confirm RN transport logging still works and consoleLogLevel="silent" suppresses logs once init is wired up

Package updates

  • @crossmint/client-sdk-rn-window: patch (changeset added)

Link to Devin run
Requested by: Guille (@xmint-guille)

@devin-ai-integration
Copy link
Contributor Author

Original prompt from Guille
we introduced the following change here to supress logs in our sdks:
https://github.com/Crossmint/crossmint-sdk/commit/27194e5751fd4bb3afb190550382760f978d7a20

i've tried this with the following code snippet, and I can still see logs once I'm logged in:
```ts
"use client";

import {
  CrossmintProvider,
  CrossmintAuthProvider,
  CrossmintWalletProvider,
  useAuth,
  useWallet,
} from "@crossmint/client-sdk-react-ui";

function WalletUser() {
  const { login, user } = useAuth();
  const { wallet, status } = useWallet();

  return (
    <div>
      {user == null ? (
        <button onClick={login}>Sign In</button>
      ) : (
        <p>
          Wallet: {wallet?.address} ({status})
        </p>
      )}
    </div>
  );
}

export default function Page() {
  return (
    <CrossmintProvider
      apiKey={process.env.NEXT_PUBLIC_CLIENT_API_KEY ?? ""}
      consoleLogLevel="silent"
    >
      <CrossmintAuthProvider>
        <CrossmintWalletProvider
          createOnLogin={{
            chain: "polygon-amoy",
            signer: { type: "email" },
          }}
        >
          <WalletUser />
        </CrossmintWalletProvider>
      </CrossmintAuthProvider>
    </CrossmintProvider>
  );
}

can you determine the issue and propose a PR in case it's feasible to maintain the silent logs flag in every part of our sdks?

</details>

@devin-ai-integration
Copy link
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI' or '@devin'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@changeset-bot
Copy link

changeset-bot bot commented Feb 20, 2026

🦋 Changeset detected

Latest commit: e0a0bca

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@crossmint/client-sdk-rn-window Patch
expo-demo Patch
@crossmint/client-sdk-react-native-ui Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

6 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment on lines +23 to +30
const platform = detectPlatform();
if (platform === "browser") {
const sink = new BrowserDatadogSink(environment);
rnWindowLogger.addSink(sink);
} else if (platform === "server") {
const sink = new ServerDatadogSink(environment);
rnWindowLogger.addSink(sink);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

missing react-native platform case. detectPlatform() returns "react-native" for RN environments, but only "browser" and "server" cases are handled here. See packages/wallets/src/logger/init.ts:46-50 for the correct pattern with a react-native case in the switch statement.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/client/rn-window/src/logger.ts
Line: 23-30

Comment:
missing `react-native` platform case. `detectPlatform()` returns `"react-native"` for RN environments, but only `"browser"` and `"server"` cases are handled here. See `packages/wallets/src/logger/init.ts:46-50` for the correct pattern with a `react-native` case in the switch statement.

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

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.

1 participant