Introduce env parameter to support sandbox and paypal #485
Conversation
🦋 Changeset detectedLatest commit: 014d1a3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Co-authored-by: Nate Bierdeman <nbierdeman@paypal.com>
| @@ -21,6 +21,7 @@ interface PayPalScriptQueryParameters { | |||
| } | |||
|
|
|||
| interface PayPalScriptDataAttributes { | |||
| env?: "sandbox" | "production"; | |||
There was a problem hiding this comment.
@ravishekhar I'm still seeing the name as env in the diff. Can we update to environment?
There was a problem hiding this comment.
Ah! I see my git push got denied earlier. Just did that again. Thanks for catching this.
… env-option # Conflicts: # packages/paypal-js/src/utils.ts
| @@ -104,6 +104,13 @@ function validateArguments(options: unknown, PromisePonyfill?: unknown) { | |||
| if (typeof options !== "object" || options === null) { | |||
| throw new Error("Expected an options object."); | |||
| } | |||
| const { environment } = options as PayPalScriptOptions; | |||
|
|
|||
| if (environment && !["production", "sandbox"].includes(environment)) { | |||
There was a problem hiding this comment.
I prefer to remove this validation code for two reasons:
- The logic in utils.ts will ignore any bad values and will default the production environment.
- paypal-js is designed to compile to ES3 so using Array.prototype.includes() would be a breaking change. If we really want to keep this, let's replace the usage of includes() with something that transpiles nicely to ES3 and does not require a polyfill.
This loader is meant to be as small as possible on top of the PayPal JS SDK.
There was a problem hiding this comment.
I agree with the challenges with Array.prototype.includes() . We can definitely use an alternative there 👍 .
About not having validations on input, once production stops the support for sandbox, then this may become a source of errors, if there is any typos in the input options.
There was a problem hiding this comment.
I see your point. Let's replace the usage of Array.prototype.includes() with some old school JS then :)
loadScriptmethod to allow passingenv = sandbox or production.env=sandboxis passed as an option, then the sdk script will be loaded fromwww.sandbox.paypal.com/sdk/js