Skip to content
New issue

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

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

Already on GitHub? # to your account

feat: rewrite enumerated literals as Enums #633

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

juancarlospaco
Copy link
Contributor

What kind of change does this PR introduce?

  • Rewrite enumerated string literals as actual Enums.

@grdsdev
Copy link
Contributor

grdsdev commented Nov 27, 2024

@juancarlospaco what's the benefits of using the StrEnum instead of Literal?

@juancarlospaco juancarlospaco marked this pull request as ready for review November 27, 2024 12:13
@juancarlospaco
Copy link
Contributor Author

It can check if the value is in the range of the enum, for extra safety.
eg. Provider("INVALID") will complain etc.

Enum can be a more correct representation of "enumerated string values".

juancarlospaco and others added 3 commits December 13, 2024 14:04
Co-authored-by: Joel Lee <lee.yi.jie.joel@gmail.com>
Copy link
Contributor

@silentworks silentworks left a comment

Choose a reason for hiding this comment

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

I'm going to leave this PR parked as when I test it locally in my existing app which makes use of the literals for type hint it breaks the type checking. This is a big breaking change and should be done at the next major version change.

@silentworks
Copy link
Contributor

Example of breaking changes:

# this is now broken because "provider" is expecting a enum class instance
resp = await supabase.auth.link_identity(
        {
            "provider": "google",
            "options": {"redirect_to": f"{request.base_url}auth/callback"},
        }
    )

now becomes

resp = await supabase.auth.link_identity(
        {
            "provider": Provider("google"),
            "options": {"redirect_to": f"{request.base_url}auth/callback"},
        }
    )

And a more full FastAPI function that uses hinting inside of the params

async def confirm(
    request: Request,
    token_hash: str,
    type: EmailOtpType = "email",
    supabase: AClient = Depends(create_supabase),
):
    try:
        if token_hash and type:
            if type == "recovery":
                request.session["password_update_required"] = True
            await supabase.auth.verify_otp(
                params={"token_hash": token_hash, "type": type}
            )

        return {"message": "User signed in successfully."}
 ...

now becomes

async def confirm(
   request: Request,
   token_hash: str,
   type: EmailOtpType | None = None,
   supabase: AClient = Depends(create_supabase),
):
   try:
       if not isinstance(type, EmailOtpType):
           type = EmailOtpType("email" if type is None else type)

       if token_hash and type:
           if type.value == "recovery":
               request.session["password_update_required"] = True
           await supabase.auth.verify_otp(
               params={"token_hash": token_hash, "type": type}
           )

       return {"message": "User signed in successfully."}
...

@juancarlospaco
Copy link
Contributor Author

I agree with the parking, it can sleep for a while. 👍

@grdsdev grdsdev added this to the v3 milestone Jan 7, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants