chore: allow extra parameters to pass through to the bridge url #220
Conversation
gregjopa
force-pushed
the
allow-extra-params-for-bridge-url
branch
from
4f01f79
to
4324d38
Compare
| throw new Error(`Expected script url to be ${sdkUrl} - got ${src}`); | ||
| } | ||
| if (uid !== sdkUID) { | ||
| throw new Error(`Expected data UID be ${sdkUID} - got ${uid}`); |
There was a problem hiding this comment.
i was about to say "you should use an expect for this" but i just realized what repo we are in 😂
| } | ||
|
|
||
| // supported query string parameters | ||
| const { origin, version, ["payment-flow"]: paymentflow, debug } = query; |
There was a problem hiding this comment.
Do we still want or need to maintain the list of required parameters?
There was a problem hiding this comment.
I suppose this allows whoever is using unpackSDKMeta to catch these errors, whereas it would be harder to detect that the SDK server responded with a validation error.
There was a problem hiding this comment.
When would this line of code error? I may be missing something. Best I can tell, query should always be an object. And if one of these values is not defined, it would not error:
const query = {};
const { origin, version } = query;
console.log(origin) // => undefinedI added this destructuring just for readability. The code below it contains the checks for if the values are required or not (ex: if (version === undefined);
| query["payment-flow"] === undefined || | ||
| !validPaymentFlows.includes(query["payment-flow"]) | ||
| ) { | ||
| if (paymentflow === undefined || !validPaymentFlows.includes(paymentflow)) { |
There was a problem hiding this comment.
Would it be better to do this validation on the server-side as well?
There was a problem hiding this comment.
I'm open to removing it here.
|
We chatted through this PR. Going to close this out as unmerged. |
This PR updates the web sdk bridge logic to allow for extra parameters to be passed through. These extra parameters will be validated and accepted/rejected at a different step outside this codebase. The purpose of this change is to make updating sdk-client easier in the future for the web sdk.