token retrieval without init #102
Conversation
ashleysmithTTD
changed the title
| 'Expected getAdvertisingToken() returns undefined and isLoginRequired() returns undefined', | ||
| }; | ||
| }, | ||
|
|
There was a problem hiding this comment.
This state was only used in one test, and isn't really relevant anymore since an advertising token can be found without init
| } | ||
| return null; | ||
| } | ||
|
|
There was a problem hiding this comment.
Spoke with Lionell about making this a separate function from loadIdentityFromCookie in the CookieManager since we don't want them doing any setting (including migrating and setting the legacy cookie) without init, just getting
src/cookieManager.ts
Outdated
| return decodeURIComponent(payload.split('=')[1]); | ||
| } | ||
| } | ||
| return getCookie(this._cookieName); |
There was a problem hiding this comment.
Not sure if this is conventional or not to have these two functions be the same name - one inside the class and one outside the class - happy to change the name if needed
There was a problem hiding this comment.
I don't love it. It obviously works but it adds a little extra confusion.
src/storageManager.ts
Outdated
| this._opts.useCookie === false && | ||
| this._localStorageManager.loadIdentityFromLocalStorage() | ||
| ) { | ||
| if (!this._opts.useCookie && this._localStorageManager.loadIdentityFromLocalStorage()) { |
There was a problem hiding this comment.
this._opts.useCookie will always be false here. It's checking for true and returning above on line 61.
There was a problem hiding this comment.
It can also be undefined and if it is undefined, we would still want the cookie to be removed I believe
There was a problem hiding this comment.
sure. but above it's checking if (this._opts.useCookie) so if (!this._opts.useCookie) will necessarily be true at this point in the code. The previous code was specifically looking for 'false.' If you just remove the check it should also execute when undefined.
| !hasExpired(this._identity.refresh_expires) | ||
| ) | ||
| private temporarilyUnavailable(identity: Identity | OptoutIdentity | null | undefined) { | ||
| if (!identity && this._apiClient?.hasActiveRequests()) return true; |
There was a problem hiding this comment.
Can you just put a comment in here indicating that this means that the identity is expired but refreshable? Might help the next person.
src/cookieManager.ts
Outdated
| return decodeURIComponent(payload.split('=')[1]); | ||
| } | ||
| } | ||
| return getCookie(this._cookieName); |
There was a problem hiding this comment.
I don't love it. It obviously works but it adds a little extra confusion.
ashleysmithTTD
deleted the
ans-UID2-3578-allow-token-retrieval-without-init
branch
No description provided.