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

Comments and minor improvements for utils.ts #83

Open
wants to merge 16 commits into
base: staging
Choose a base branch
from

Conversation

vnnkl
Copy link

@vnnkl vnnkl commented Sep 6, 2023

Added some comments to the function to make them more consistent with the others

The isObj function in the utils.ts file has been updated to provide more accurate type checking. Instead of simply checking if the value is an object, it now checks if it is an object with a string key and unknown value. This change improves the reliability of type checking in the codebase because in TypeScript, null is considered an object, which can lead to unexpected behavior.

Made checkResponseError more concise

Constantin Vennekel added 3 commits September 6, 2023 10:58
Refactored the checkResponseError function to handle Axios errors more effectively. Now, if an error response contains either an "error" or a "detail" property, it will throw an Error with the corresponding message. This change improves the error reporting and makes the code more robust.

Closes cashubtc#123
The `isObj` function in the `utils.ts` file has been updated to provide more accurate type checking. Instead of simply checking if the value is an object, it now checks if the value is an object with a string key and unknown value. This change improves the reliability of type checking in the codebase, because in TypeScript, null is considered an object, which can lead to unexpected behavior.
- added explainer comments
- made check response error more concise
Copy link
Collaborator

@gandlafbtc gandlafbtc left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

Sorry i got to it so late.

We removed axios, do you want to adapt the PR?
I think you can remove the changes in the checkResponseError function. No need to change anything there, we want to revamp the error handling anyway

Constantin Vennekel added 2 commits October 9, 2023 08:33
changes:
- Exported `splitAmount`, `getDefaultAmountPreference`, `bytesToNumber`, `hexToNumber`, and `bigIntStringify` functions.
- Added JSDoc comments for new functions: `isPowerOfTwo`, `getPreference`, and other utility functions.
- Improved code readability by adding line breaks and indentation.
Copy link
Collaborator

@gandlafbtc gandlafbtc left a comment

Choose a reason for hiding this comment

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

Hi! Can you please add the missing return types? I think then after this we merge it. I'm still waiting on feedback on other PRs, but IDK how long this takes, so let's merge this first

src/utils.ts Outdated
* Checks if a number is a power of two.
* @param number The number to check
* @returns True if the number is a power of two, false otherwise
*/
function isPowerOfTwo(number: number) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

while we're at it, can you add the return type to this function signature?

src/utils.ts Outdated
* @returns
* Derives the keyset id from a set of keys.
* @param keys The keys to derive the keyset id from
* @returns The derived keyset id
*/
export function deriveKeysetId(keys: MintKeys) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add the missing return type here?

src/utils.ts Outdated
* Sorts an array of proofs by id.
* @param proofs The proofs to sort
* @returns The sorted proofs
*/
export function sortProofsById(proofs: Array<Proof>) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

and here

gandlafbtc and others added 8 commits February 20, 2024 10:49
Added try-catch block to handle errors when getting preferences. Log error message if an exception occurs during preference retrieval.
- Refactored code to separate prefix removal logic into a new function. Updated getDecodedToken to utilize this function for removing prefixes before handling tokens.
and invoice decoding functions. Add type annotations and cleanup code.
The isPowerOfTwo function in utils.ts was updated to include type annotations for better type safety. The return value now explicitly states the boolean type.
@vnnkl vnnkl requested a review from gandlafbtc April 12, 2024 15:01
Constantin Vennekel added 2 commits April 12, 2024 17:08
The function bigIntStringify now returns a stringified value using JSON.stringify for non-bigint values.
@vnnkl vnnkl changed the base branch from main to staging April 22, 2024 09:58
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