Skip to content

Conversation

@xinatcg
Copy link

@xinatcg xinatcg commented Jun 18, 2025

@novabyte novabyte requested review from Copilot and sesposito June 18, 2025 13:40
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances the RPC methods to support pure string inputs and outputs, reducing JSON serialization overhead when raw data is used.

  • Extend RpcResponse.payload to accept string as well as object
  • Add rawInput/rawOutput flags to rpc and rpcHttpKey methods
  • Adjust serialization/deserialization to conditionally parse or pass strings directly
Comments suppressed due to low confidence (4)

packages/nakama-js/client.ts:1742

  • The rpc method signature was extended with rawInput and rawOutput flags but the JSDoc comment above it wasn’t updated. Consider documenting these new parameters to explain their purpose and default values.
  async rpc(session: Session, id: string, input: object | string, rawInput: boolean = false, rawOutput: boolean = false): Promise<RpcResponse> {

packages/nakama-js/client.ts:1757

  • The rpcHttpKey method signature now includes rawInput and rawOutput but its JSDoc remains generic. Please update the doc comment to describe how these flags affect input serialization and response parsing.
  async rpcHttpKey(httpKey: string, id: string, input?: object | string, rawInput: boolean = false, rawOutput: boolean = false): Promise<RpcResponse> {

packages/nakama-js/client.ts:1747

  • This branch introduces conditional serialization logic for raw vs. JSON inputs. Consider adding unit tests to cover both rawInput = true and rawInput = false scenarios to ensure correct behavior.
    const inputStr = rawInput ? input as string : JSON.stringify(input);

packages/nakama-js/client.ts:1765

  • The catch block simply rethrows the error without adding context. You can remove the .catch entirely and let the promise propagate, or enhance error handling with additional diagnostics if needed.
      }).catch((err: any) => {

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