feat: improve function return types

[alaister]

Fixes #55.

What is the current behaviour?

There are some type errors, and functions return everything with | null, making it difficult for end-users to work with.

What is the new behaviour?

Functions now return type unions, which are more correct and easier to work with.

Additional context

I've labelled this as feat instead of chore as it should be treated as a breaking change because it may show new type errors to existing users.

[Jaaneek]

  ): Promise<
    | {
        data: { message: string }
        error: null
      }
    | {
        data: null
        error: Error // in future we might add Error || OurOwnError
      }
> {
     const data = something
     return { data, error: null }
    } catch (error) {
        if(basicErrorTypeguard(error)) return { data: null, error }
        // In case we would need to add new Error type in future.
        //if(ourOwnErrorTypeguard(error)) return { data: null, error }
        throw error
    }
  }

Reasoning:

1. If there will be any other error that is not handled by us, we shouldn't return it without a type.
2. User won't be able to handle it, returning an unknown type as error isn't an option as well as that makes developer experience completely worthless. We might as well return a flag 'isError'

Why this solution:
Typeguard will ensure that we only ever return our 'Error' and users can handle it.
They will get an error in development that will give them information.
Bonus:
Backward compatibility.
With this code it's possible to add new ErrorTypes in future without breaking anything. Its maintanable

Another possible solution (not recommended):

catch (error) {
    const castedError = error as Error
      return { data: null, error }
    }

This will also work, but it will tell the developer that the error is of type 'Error'.
If any other error is thrown, the developer won't get information about it. This is bug prone.
For example:

1. Developer shows information from the error to user (user of website).
2. Any other error is thrown that doesn't match 'Error'.
3. Javascript might crash because of accessing a property that doesn't exist.

[alaister]

Thanks, @Jaaneek!

I agree with your first option more than the second.

The only thing I don't feel great about is returning an error in the object, but having the function also be able to throw... If a user sees that error in the object, my intuition is that all errors would come through that error and not through a try/catch.

I'm keen to hear your thoughts!

[Jaaneek]

@alaister

My proposal:

Create custom error that will format any unknown error
Something like

try {
  throw new Error('Some supabase error')
} catch (error) {
  if (error instanceof Error) return error
// if(error instaceof SomeOtherError) return error
// if(error instaceof AnotherError) return error

return toStorageError(error,Reason.Unknown)
// Might be: return toStorageError(error,Reason.UpdateFail)
}

const toStorageError = (error:unknown,reason:Reason) => {
if(isErrorWithMessage) return new StorageError(error.message,reason,error)


// We might aswell do:
// return new StorageError('',reason,error)
try {
    return new StorageError(JSON.stringify(maybeError),reason,error)
  } catch {
    // fallback in case there's an error stringifying the maybeError
    // like with circular references for example.
    return new StorageError(String(maybeError),reason,error)
  }
}

export class StorageError extends Error {
  reason: Reason
  originalError:unknown
  constructor(message: string, reason: Reason,originalError:unknown) {
    super(message)
    this.name = 'StorageError'
    this.Reason= status
    this.,originalError: originalError
  }


  }
}

enum Reason {
  Unknown,
  OtherEasons...
}

function isErrorWithMessage(error: unknown): error is ErrorWithMessage {
  return (
    typeof error === 'object' &&
    error !== null &&
    'message' in error &&
    typeof (error as Record<string, unknown>).message === 'string'
  )
}

Why?
It'simportant to let the developer handle the final error.
With introducing 'StorageError' we will let developers always:

• See a message
• We will let him know the reason behind the error (if we know of that)
• Let him handle the final error which will be always unknown.

Suggestions
Name it better, 'StorageError ' might be understood as something that we throw, which is not. We are just wrapping it for developers.

More to read about this pattern
https://kentcdodds.com/blog/get-a-catch-block-error-message-with-typescript

[mircea-pavel-anton]

I think that PRs #62 and #63 could also be closed and added here, as they also address the return types for some functions.

[alaister]

@mirceanton, I'll review your PRs now and try and get them in before this goes out. Thank you!

[kiwicopple]

🎉 This PR is included in version 1.8.0-next.1 🎉

The release is available on:

• GitHub release
• npm package (@next dist-tag)

Your semantic-release bot 📦🚀

[kiwicopple]

🎉 This PR is included in version 2.0.0-rc.1 🎉

The release is available on:

• GitHub release
• npm package (@rc dist-tag)

Your semantic-release bot 📦🚀