search

square / react-native-square-reader-sdk

React Native Plugin for Square Reader SDK
Apache License 2.0
115 stars 34 forks source link

Fix Typescript Support #199

Closed caleb-harrelson closed 1 year ago

caleb-harrelson commented 1 year ago

Summary

The existing TypeScript support, added via PR #148, is broken. The types aren't available when consumed, are incorrect anyway, and the PR contained breaking changes.

This PR reverts everything in #148 and adds TypeScript support without changing any code, via a manually crafted index.d.ts file, which is referenced from package.json so that consumers get the types and documentation that comes with it. Documentation came from the existing documentation in this repo, including a small typo fix to it.

Note: since this is a manually crafted file, it should be considered documentation and updated whenever public APIs are changed. If a change is necessary for reference.md it's most likely necessary in index.d.ts too.

Related issues

Fixes broken PR #148

Changelog

Test Plan

I've been using this declarations file in my working TypeScript code for a long time. I learned yesterday that #148 "added TypeScript support" but also broke the code, and was deployed as part of a version update I hadn't consumed until this week. PR #198 fixes the broken code in the least invasive way possible, while this PR addresses the TypeScript not actually working part, without the code modifications done by #148.

I tested this PR by removing the declarations file from my project and making the same changes to the node_modules/react-native-square-reader-sdk directory that I've made here, then ensuring that TypeScript types and documentation were still functional.

hukid commented 1 year ago

Thanks for helping maintain this repo, @caleb-harrelson! If we fix the tipSettings issue, do you think we need this fix? What else is broken in the current version?

caleb-harrelson commented 1 year ago

Thanks for helping maintain this repo, @caleb-harrelson! If we fix the tipSettings issue, do you think we need this fix? What else is broken in the current version?

No functionality is broken anymore, now that the tipSettings are again optional, as long as you define broken as "makes the library not work". The TypeScript implementation is broken as it the library does not expose any TypeScript definitions. This PR would add those definitions, along with the documentation that comes with them, making it much easier for those using TypeScript to consume the package.

An alternative solution is for me to submit the definitions to the definitely-typed project, which is a place for TypeScript definitions for libraries that don't want to manage the declarations/documentation themselves. Having it in the native library is generally preferred as it gives the actual developers control over the documentation, allowing them to keep it up to date as the library evolves.