refactor: initialize the user auth state synchronously #639
Conversation
🦋 Changeset detectedLatest commit: c9db89e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
A wrangler prerelease is available for testing. You can install this latest build in your project with: npm install --save-dev https://prerelease-registry.developers.workers.dev/runs/2009253485/npm-package-wrangler-639You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.developers.workers.dev/prs/639/npm-package-wrangler-639Or you can use npx https://prerelease-registry.developers.workers.dev/runs/2009253485/npm-package-wrangler-639 dev path/to/script.js |
|
Hope this isn't urgent, I'd love to review this before it lands |
|
No not urgent but it is not as scary as you may think. |
packages/wrangler/src/user.tsx
Outdated
| /** | ||
| * Compute the current auth state. | ||
| */ | ||
| function getAuthState(): State { |
There was a problem hiding this comment.
"state" isn't super descriptive here. How about getAuthTokens()?
There was a problem hiding this comment.
I would quite like to go a step further and split apart the OAuth state processing from the external auth token state. But I thought that would be too large a change in this PR.
The OAuth state fields that are stored in State really an internal implementation thing which code outside of user.tsx should not care about.
packages/wrangler/src/user.tsx
Outdated
| /** | ||
| * Run the initialisation of the auth state, in the case that something changed. | ||
| */ | ||
| export function reinitialise(): void { |
There was a problem hiding this comment.
Would you like to take this opportunity to rename this function too? It was never clear even before. Got any ideas?
There was a problem hiding this comment.
Yeah - I changed it from initialise() to reinitialise() but I would be up for an even more description name... perhaps reinitialiseAuth()?
We can now initialize the user state synchronously, which means that we can remove the checks for whether it has been done or not in each of the user auth functions.
petebacondarwin
force-pushed
the
sync-auth-init
branch
from
5932bbf to
c9db89e
Compare
We can now initialize the user state synchronously, which means that
we can remove the checks for whether it has been done or not in each
of the user auth functions.