-
Notifications
You must be signed in to change notification settings - Fork 4
feat(auth): use the optional service pattern for auth-dependent layers #1347
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
base: main
Are you sure you want to change the base?
Conversation
| export const layer = (url: string) => | ||
| Layer.effect(Admin)( | ||
| Effect.gen(function*() { | ||
| const auth = yield* Effect.serviceOption(Auth.Auth) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cmwhited - FYI this is what I was referring to by the "optional service pattern"
|
Does this PR re-introduce the issue with the Node.js dependencies? Just making sure we don';t break browser usage again. |
|
LocalCache.ts has a dependency on |
|
I was able to run the studio without issue, but perhaps I can sync up with @cmwhited when there's time (maybe after you all are back) to understand if there's something I missed / a more thorough way I can test to ensure all works properly. |
|
@IMax153 are you blocked here? What do you want to do with this PR? |
Hey @leoyvens - @cmwhited is going to do a secondary validation that this PR does not affect the functionality of Amp Studio that @fubhy mentioned above either today or tomorrow. As long as everything is working properly, we should be good to merge this after that. |
Auth.tsand intoModel.tsRedactedfor sensitive tokensURLinterpolationAdminserviceAuth<->Admin, so you can just provideAuthtoAdminwhen you want / need authenticated requests