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

fix(types): make build and parse generic #594

Merged

Conversation

sarahdayan
Copy link
Contributor

@sarahdayan sarahdayan commented Jun 23, 2023

Purpose / Goal

This PR improves typing for build and parse by making both functions generic.

It also fixes the return type of build, which should be string.

parse

// Before
const result = xml.parse(data); // `result` is `any`

// After
type ExpectedObject = {
  title: string;
  description: string;
};

const result = xml.parse<ExpectedObject>(data); // `result` is `ExpectedObject`

build

// Before
const xml = builder.build(obj); // `xml` is `any`

// After
const xml = builder.build(obj); // `xml` is `string`

Type

Please mention the type of PR

  • Bug Fix
  • Refactoring / Technology upgrade
  • New Feature

@amitguptagwl
Copy link
Member

Can you please explain me your changes for parse function? What exactly we're expecting

@sarahdayan
Copy link
Contributor Author

@amitguptagwl This is only a type change. parse used to return any, forcing you to cast it. Now you can provide a generic type to maintain type-safety, which is usually preferrable as many linters frown upon type casting.

@amitguptagwl
Copy link
Member

What I can understand here is to define the return type of parser to ExpectedObject with properties title and description which is not true for parser. Hence, I'm confusd what exactly you mean with the type you defined.

@equipatuequipo
Copy link

In the example they gave the type ExpectedObject has the properties title and description but in a real world use that type have the properties that you expect the parsed XML to have.
For example for this XML:

<person>
     <name>John</name>
     <surname>Doe</surname>
</person>

the corresponding Typescript type would be:

type Person = {
    name: string;
    surname: string;
}

and you would use it like:

const result = xml.parse<Person>(data);

This would make the result variable have the type Person instead of any which is much better in terms of type safety.
You can read more about generics here.

@amitguptagwl amitguptagwl merged commit 75acbc5 into NaturalIntelligence:master Sep 20, 2023
4 checks passed
amitguptagwl added a commit that referenced this pull request Sep 24, 2023
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.

None yet

3 participants