fix: resolves window.open issue in safari due to async storage call (#…

…620) * bug: resolves window.open issue in safari due to async storage call * updating tests for new flow
dfinity · Aug 31, 2022 · 4141df2 · 4141df2
1 parent 129293d
commit 4141df2
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 25 deletions.
diff --git a/docs/generated/changelog.html b/docs/generated/changelog.html
@@ -21,6 +21,10 @@ <h2>Version 1.0.0</h2>
           </li>
         </ul>
       </ul>
+      <h3>Version 0.13.3</h3>
+        <ul>
+          <li>Auth Client moves key fallback generation to the create method instead of login and makes the _key non-nullable. This fixes a regression with async window.open behavior in Safari</li>
+        </ul>
       <h2>Version 0.13.2</h2>
       <ul>
         <li>auth-client avoids localstorage global and can be used in a web worker or nodejs</li>

diff --git a/packages/auth-client/src/index.test.ts b/packages/auth-client/src/index.test.ts
@@ -594,7 +594,8 @@ describe('Migration from localstorage', () => {
 
     await AuthClient.create({ storage });
 
-    expect(storage.set as jest.Mock).toBeCalledTimes(0);
+    // Key is stored during creation when none is provided
+    expect(storage.set as jest.Mock).toBeCalledTimes(1);
   });
   it('should not attempt to migrate if a delegation is already stored', async () => {
     const storage: AuthClientStorage = {
@@ -609,7 +610,7 @@ describe('Migration from localstorage', () => {
 
     await AuthClient.create({ storage });
 
-    expect(storage.set as jest.Mock).toBeCalledTimes(0);
+    expect(storage.set as jest.Mock).toBeCalledTimes(1);
   });
   it('should migrate storage from localstorage', async () => {
     const localStorage = new LocalStorage();
@@ -624,18 +625,6 @@ describe('Migration from localstorage', () => {
 
     await AuthClient.create({ storage });
 
-    expect(storage.set as jest.Mock).toBeCalledTimes(2);
-    expect((storage.set as jest.Mock).mock.calls).toMatchInlineSnapshot(`
-      Array [
-        Array [
-          "delegation",
-          "test",
-        ],
-        Array [
-          "identity",
-          "key",
-        ],
-      ]
-    `);
+    expect(storage.set as jest.Mock).toBeCalledTimes(3);
   });
 });
diff --git a/packages/auth-client/src/index.ts b/packages/auth-client/src/index.ts
@@ -256,12 +256,18 @@ export class AuthClient {
       ? undefined
       : IdleManager.create(options.idleOptions);
 
+    if (!key) {
+      // Create a new key (whether or not one was in storage).
+      key = Ed25519KeyIdentity.generate();
+      await storage.set(KEY_STORAGE_KEY, JSON.stringify(key));
+    }
+
     return new this(identity, key, chain, storage, idleManager, options);
   }
 
   protected constructor(
     private _identity: Identity,
-    private _key: SignIdentity | null,
+    private _key: SignIdentity,
     private _chain: DelegationChain | null,
     private _storage: AuthClientStorage,
     public readonly idleManager: IdleManager | undefined,
@@ -378,14 +384,6 @@ export class AuthClient {
      */
     onError?: ((error?: string) => void) | ((error?: string) => Promise<void>);
   }): Promise<void> {
-    let key = this._key;
-    if (!key) {
-      // Create a new key (whether or not one was in storage).
-      key = Ed25519KeyIdentity.generate();
-      this._key = key;
-      await this._storage.set(KEY_STORAGE_KEY, JSON.stringify(key));
-    }
-
     // Set default maxTimeToLive to 8 hours
     const defaultTimeToLive = /* hours */ BigInt(8) * /* nanoseconds */ BigInt(3_600_000_000_000);
 
@@ -493,7 +491,6 @@ export class AuthClient {
 
     // Reset this auth client to a non-authenticated state.
     this._identity = new AnonymousIdentity();
-    this._key = null;
     this._chain = null;
 
     if (options.returnTo) {