Optimize - Update Proposition with callback support

[akhiljain1907]

Added optional callback in update proposition api with onSuccess and onError to provide propositions as well as error(if any). Public Interface looks like-

updatePropositions: (
decisionScopes: Array,
xdm?: Map<string, any>,
data?: Map<string, any>,
onSuccess?: (response: Map<string, Proposition>) => void,
onError?: (error: AEPOptimizeError) => void
) => void;

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• I have signed the Adobe Open Source CLA.
• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.

[namArora3112]

Can we replace the hardcoded number literal with a constant that has a meaningful name? This will help improve code readability and make it easier for everyone to understand its purpose.

[namArora3112]

Let's extract these inline styles into a new codeText style object inside the styles file.
codeText: { fontFamily: 'monospace', backgroundColor: '#f0f0f0', padding: 10, },

[namArora3112]

Instead of mutating response.propositions directly, consider creating a new object using an immutable pattern. This helps avoid potential side effects from modifying native responses.

const finalResponse: UpdatePropositionsResponse = {
  ...response,
  propositions: response.propositions
    ? Object.fromEntries(
        Object.entries(response.propositions).map(
          ([key, value]) => [key, new Proposition(value)]
        )
      )
    : undefined
};
callback(finalResponse);

[namArora3112]

If there’s a possibility of null or undefined values, JSON.stringify might throw an error. You could consider wrapping it in a try-catch block for safety. But if you're certain the data will always be valid, it should be fine as is.

[navratan-soni]

1. Annotate Callback as @nullable parameter here.
2. Callback is now a mandatory parameter here, it needs to be passed as null if no usage is needed to user. Can this be refactored like this, if the API is being changed.
   public void updatePropositions(final ReadableArray decisionScopesArray, ReadableMap xdm, ReadableMap data) { updatePropositions(decisionScopesArray, xdm, data, null) }
   So both the APIs are available one with callback, and one without callback ?

[namArora3112]

please remove this before merging to staging

[navratan-soni]

How are we handling the case, where map is null or empty ?

[akhiljain1907]

In that case it will return empty map.

[ishita-gambhir-adobe]

Should we move this to RCTAEPOptimizeUtil

[akhiljain1907]

Done

[ishita-gambhir-adobe]

Reviewed the JS and Android sections. LGTM
[nit] Please move the helper functions to util classes.

[yangyansong-adbe]

IMO, it's better to return a Promise here. I also asked Copilot to share some thoughts.

Returning a Promise vs. Using Callbacks

Returning a Promise is generally better for modern JavaScript/TypeScript code because:

• It allows use of async/await, making code easier to read and maintain.
• It integrates more naturally with promise-based workflows and error handling (try/catch).
• It enables chaining and composition with other asynchronous operations.

Callback-based Approach is more traditional:

• It works well if you need to support environments where Promises aren’t available.
• However, it can lead to "callback hell" and less readable code when chaining multiple async operations.

Callback Style (current selection):

updatePropositions(scopes, xdm, data, (result) => {
  // handle result
}, (error) => {
  // handle error
});

Promise Style (recommended):

updatePropositions(scopes, xdm, data)
  .then(result => { /* handle result */ })
  .catch(error => { /* handle error */ });

Conclusion:
Yes, returning a Promise is typically better for usability, readability, and consistency with modern async patterns. You could refactor updatePropositions to return a Promise that resolves to the response or rejects with an error, instead of using callbacks. This would make the API more intuitive for developers using async/await.

[spoorthipujariadobe]

We discussed offline about the limitations that @akhiljain1907 mentioned around using Promise based approach here because of the requirement to invoke both the call and fail method asynchronously for the same single request. @yangyansong-adbe bought up a very good point that it would be hard for customers to manage this with callbacks on single-threaded javascript applications. How do you feel about using Promise and providing an array as a response, where you can have individual result or error for each decision scope in the request?

[akhiljain1907]

Hi @yangyansong-adbe and @spoorthipujariadobe, thanks a lot for reviewing.

I also wanted to use a promise-based approach and I completely agree that, in general, returning a promise is cleaner than providing callbacks.

However, after discussing with Spoorthi, here’s the limitation we ran into: in this API’s case, both the “call” and “fail” responses can occur asynchronously and independently. That means we might receive:

1. only a call result
2. only a fail result
3. both call and fail results (in any order)

If we used a single promise, we could only resolve or reject once, which doesn’t work for scenarios where both outcomes can be present. The alternative would be to wait for both to complete and then resolve with an object containing both results — but that would require holding the API response until a timeout is reached, which:

isn’t efficient (especially if one result arrives early)

doesn’t work well here since React Native doesn’t currently expose a timeout configuration for this API (default is ~10s, which is too long to delay returning results)

Because of these constraints, I went with two separate callbacks for success and error to ensure the caller gets results as soon as they’re available without unnecessary waiting.

[yangyansong-adbe]

Hi @yangyansong-adbe and @spoorthipujariadobe, thanks a lot for reviewing.

I also wanted to use a promise-based approach and I completely agree that, in general, returning a promise is cleaner than providing callbacks.

However, after discussing with Spoorthi, here’s the limitation we ran into: in this API’s case, both the “call” and “fail” responses can occur asynchronously and independently. That means we might receive:

1. only a call result
2. only a fail result
3. both call and fail results (in any order)

If we used a single promise, we could only resolve or reject once, which doesn’t work for scenarios where both outcomes can be present. The alternative would be to wait for both to complete and then resolve with an object containing both results — but that would require holding the API response until a timeout is reached, which:

isn’t efficient (especially if one result arrives early)

doesn’t work well here since React Native doesn’t currently expose a timeout configuration for this API (default is ~10s, which is too long to delay returning results)

Because of these constraints, I went with two separate callbacks for success and error to ensure the caller gets results as soon as they’re available without unnecessary waiting.

A single Promise can’t represent that because it can only resolve or reject once. So, I don't think there are any straightforward approaches to return a Promise in this case. I agree that the dual-callbacks is the simplest approach.