Set data-js-sdk-library default value #465
Conversation
🦋 Changeset detectedLatest commit: 9d90088 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 |
| @@ -19,6 +19,14 @@ export function loadScript( | |||
| if (typeof document === "undefined") return PromisePonyfill.resolve(null); | |||
|
|
|||
| const { url, attributes } = processOptions(options); | |||
|
|
|||
| if ( | |||
There was a problem hiding this comment.
I'd prefer to skip the validation on the existing value and only set it if the data attribute value is undefined. That way we can add new libraries in the future like vue-paypal-js and not have to worry about updating this code path.
if (!attributes["data-js-sdk-library"]) {
attributes["data-js-sdk-library"] = "paypal-js";
}There was a problem hiding this comment.
Okay, cool. I'll need to look into this, because it ends up inserting a new script in the DOM even when there's already one with the same params.
There was a problem hiding this comment.
@gregjopa, I tried adding the data-js-sdk-library default value in loadScript, processOptions, loadCustomScript, and insertScriptElement, and ended up moving it back to loadScript. Definitely open to trying something else too, but marking this ready for review for now.
There was a problem hiding this comment.
I think you added it in the right spot in loadScript() @nbierdeman 💯
nbierdeman
force-pushed
the
DTPPCPSDK-1482
branch
from
1729c28 to
01cb0bc
Compare
nbierdeman
changed the title
| @@ -64,6 +64,10 @@ export function loadCustomScript( | |||
|
|
|||
| const { url, attributes } = options; | |||
|
|
|||
| if (attributes && !attributes["data-js-sdk-library"]) { | |||
There was a problem hiding this comment.
loadCustomScript will always have attributes when called by react-paypal-js/paypal-js -> loadScript, but checking for attributes here, since react-paypal-js calls loadCustomScript without attributes in getBraintreeNamespace here.
| @@ -13,6 +13,7 @@ <h1>Load Cached Script</h1> | |||
| const options = { | |||
| clientId: "test", | |||
| dataPageType: "checkout", | |||
| dataJsSdkLibrary: "paypal-js", | |||
There was a problem hiding this comment.
This smells funky, because dataJsSdkLibrary is not a required script option, but if we don't add it here, the "load cached script" tests fail.
Description
In service of DTPPCPSDK-1482, this PR sets a data-js-sdk-library value for tracking library usage.