Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated occurrences of String to string #152

Merged
merged 1 commit into from
May 13, 2024

Conversation

ajaysubra
Copy link
Collaborator

Description

In TypeScript, string is a primitive type, while String is a global object that is a wrapper around the primitive string type. We were using both of these interchangeably which could lead to issues like this -

let existingEmail = Klaviyo.getEmail(undefined);
    Klaviyo.setProfile({
      email: existingEmail, //this is an error: TS2322: Type String | null is not assignable to type string | undefined
    });

The above error is because getEmail returns an instance of type String whereas setProfile accepts an instance of Profile whose email property is of type string.

The fix was to across the board using string instead of String. This in some cases could be a breaking change if developer made any provision in their code to handle this inconsistency. Since we are in beta we will release this change as a minor version update or roll it into our first major version release.

Check List

  • Are you changing anything with the public API?
  • Are your changes backwards compatible with previous SDK Versions?

Changelog / Code Overview

Test Plan

Related Issues/Tickets

@ajaysubra ajaysubra marked this pull request as ready for review May 9, 2024 17:15
@ajaysubra ajaysubra requested a review from a team as a code owner May 9, 2024 17:15
@ajaysubra ajaysubra requested review from kennyklaviyo and removed request for a team May 9, 2024 17:15
Copy link
Contributor

@evan-masseau evan-masseau left a comment

Choose a reason for hiding this comment

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

Thank you!

@ajaysubra ajaysubra merged commit 0a6ad63 into master May 13, 2024
5 checks passed
@ajaysubra ajaysubra deleted the as/chnl-6780-String-to-string branch May 13, 2024 15:10
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.

3 participants