Skip to content

Conversation

@pkowalski
Copy link

@pkowalski pkowalski commented Oct 29, 2025

Explanation

Ramps currently depends on an sdk to handle all connections to the RAMPS api. We now have 2 different offerings which each use their own SDK. All of this routing logic is done on the mobile app. If this logic were to be extended to pdapp or extension it would require rework on those codebases. Additionally many other teams already use this controller design pattern. By moving towards this we can handle all business logic in 1 controller. It lets us expose Ramps data and apis to other experiences without those experiences requiring any knowledge of business logic. Lastly it would speed up any Ramps api integrations on extension and pdapp in the future.

References

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed, highlighting breaking changes as necessary
  • I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

Note

Introduce ramps-controller package with RampsController, tests, docs, and build/tooling setup.

  • New package: packages/ramps-controller
    • Core: Implements RampsController in src/RampsController.ts and exports public API via src/index.ts.
    • Tests: Adds src/RampsController.test.ts and jest.config.js.
    • Tooling/Build: Adds package.json, tsconfig.json, tsconfig.build.json, and typedoc.json.
    • Docs/Meta: Adds README.md, initializes CHANGELOG.md, and includes LICENSE.
  • Repo config: Updates root tsconfig.build.json to integrate the new package.

Written by Cursor Bugbot for commit 4c43415. This will update automatically on new commits. Configure here.

cursor[bot]

This comment was marked as outdated.

const nativeContext = (context as unknown as string) as keyof typeof NativeContext;
const nativeConfig: NativeRampsSdkConfig = {
context: NativeContext[nativeContext] ?? NativeContext.Browser,
};
Copy link

Choose a reason for hiding this comment

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

Bug: Unsafe Context Mapping Causes Inconsistent SDK Behavior

The NativeRampsSdk initialization on line 539 maps the context value unsafely. The direct cast from context to keyof typeof NativeContext lacks runtime validation, which can result in NativeContext[nativeContext] evaluating to undefined and the SDK falling back to a default context inconsistently.

Fix in Cursor Fix in Web

};
export type RampsControllerDepositGetTransalationAction = {
type: `${typeof controllerName}:depositGetTransalation`;
handler: RampsController['depositGetTransalation'];
Copy link

Choose a reason for hiding this comment

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

Bug: Translation Typo Causes SDK Method Mismatch

The depositGetTransalation method, its action type, and messenger handler are misspelled as "Transalation" instead of "Translation." This typo creates a mismatch with the correctly named NativeQuoteTranslation type from the SDK, potentially causing issues when calling the underlying SDK method.

Additional Locations (1)

Fix in Cursor Fix in Web

@pkowalski pkowalski marked this pull request as draft October 29, 2025 20:49
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