Skip to content

Conversation

soofstad
Copy link
Owner

What does this pull request change?

  • Add support for native clients
  • Add an example for a native Android app

Why is this pull request needed?

  • Support for native clients are currently not supported

Issues related to this change

closes #223

@soofstad soofstad marked this pull request as draft July 16, 2025 10:20
@soofstad
Copy link
Owner Author

@Jorgagu
I took the liberty to checkout your branch, and run pre-commit to fix linting and ts errors.

A few comments to further mature the PR:

  • Can you please add a README in the example on how to build and run it locally?
  • Also, remove all files and code not strictly needed (for example all the splash.png images), just reduce clutter in general.
  • I think this (https://github.com/soofstad/react-oauth2-pkce/blob/feat/support-native-clients/src/authentication.ts#L64) should throw an error, not a warning.
  • Hope we can find a better name for onLoginUrlready. Perhaps nativeAppLoginCallback or something even better... 😄
  • Does the example need to work on both native AND web? I'd prefer not, so that redirectUri is always com.yourapp.oauth://callback', and loginMethod is always native.

@Jorgagu
Copy link

Jorgagu commented Jul 16, 2025

Thanks @soofstad! I’m not used to using Python tools to lint a TypeScript project 😅.

We could also add an .editorconfig file at the root of the project, so that most IDEs will follow the same file formatting rules.
Maybe we could consider using ESLint, with the stylistic plugin, to ensure proper TypeScript formatting and linting?

Regarding the points you mentioned:

  • ✔️ I will add a README for this example.
  • ✔️ Yes, we can remove all unnecessary files.
  • ✔️ I will update the console log from warning to error.
  • For the callback function name, I’m not sure we should include “native” in its name, since it could be used in other contexts in the future. I’ll think about a more generic name.
    Here are a few naming suggestions that keep it generic and clearly indicate it’s a callback:
    • onLoginUrlReadyCallback (simple and explicit)
    • handleLoginUrlCallback (oriented toward handling the callback action)
    • onAuthorizationUrlReadyCallback (more formal, fits OAuth vocabulary)
      Let me know if you have a preference, or if you see another name that fits the project style!
  • For the example, we can focus only on the “native” case to avoid confusing developers, but also show how both cases can be handled—or maybe provide a new example demonstrating both approaches in the same project.

@Jorgagu
Copy link

Jorgagu commented Jul 16, 2025

We could also add an .editorconfig file at the root of the project, so that most IDEs will follow the same file formatting rules.
Maybe we could consider using ESLint, with the stylistic plugin, to ensure proper TypeScript formatting and linting?

Sorry didn't see inside the pre commit hooks, there is biomejs which is a eslint/prettier equivalent.

Maybe we can add a note about it in a CONTRIBUTING.md file.

@soofstad
Copy link
Owner Author

soofstad commented Jul 17, 2025

In regards to

✔️ I will update the console log from warning to error.

I mean it should not just log an error, but throw one, as it is an fatal exception.

In regards to ESLint, I've been using it for a long time, on many projects, but have found that Biome (which this project uses now via pre-commit) to be far superior in every way 😄

Create name suggestions. I'm leaning towards a mashup of your proposals; handleAuthorizationUrlCallback.

It makes it clear that you need to handle the url, instead of the library, it uses the correct name for the url, and makes it clear that it is a callback, and require a function as input.

Also, please make further PR's into this branch. Makes it easier to collaborate 🙂

Copy link

Stale pull request message

@soofstad soofstad force-pushed the feat/support-native-clients branch from a0ac65a to a4e756e Compare October 10, 2025 11: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.

💡 [REQUEST] - Add loginMethod: 'native' to support RFC 8252 (in-app/external browser + deep‑link callback)

2 participants