fix(cookies): respect the options passed into cookies when testing to see if they're enabled (#294)

* fix(cookies): respect the options passed into cookies when testing to see if they're enabled

* some shuffling around

Author: kelvin-lu

src/base-cookie.js

@@ -23,10 +23,10 @@ const get = (name) => {
 };
 
 const set = (name, value, opts) => {
-  let expires = value !== null ? opts.expirationDays : -1 ;
+  let expires = value !== null ? opts.expirationDays : -1;
   if (expires) {
     const date = new Date();
-    date.setTime(date.getTime() + (expires * 24 * 60 * 60 * 1000));
+    date.setTime(date.getTime() + expires * 24 * 60 * 60 * 1000);
     expires = date;
   }
   let str = name + '=' + value;
@@ -46,15 +46,14 @@ const set = (name, value, opts) => {
   document.cookie = str;
 };
 
-
 // test that cookies are enabled - navigator.cookiesEnabled yields false positives in IE, need to test directly
-const areCookiesEnabled = () => {
+const areCookiesEnabled = (opts = {}) => {
   const uid = String(new Date());
   try {
     const cookieName = Constants.COOKIE_TEST_PREFIX + base64Id();
-    set(cookieName, uid, {});
+    set(cookieName, uid, opts);
     const _areCookiesEnabled = get(cookieName + '=') === uid;
-    set(cookieName, null, {});
+    set(cookieName, null, opts);
     return _areCookiesEnabled;
   } catch (e) {}
   return false;
@@ -63,5 +62,5 @@ const areCookiesEnabled = () => {
 export default {
   set,
   get,
-  areCookiesEnabled
+  areCookiesEnabled,
 };

src/metadata-storage.js

@@ -10,42 +10,69 @@ import localStorage from './localstorage'; // jshint ignore:line
 import topDomain from './top-domain';
 
 /**
-* MetadataStorage involves SDK data persistance
-* storage priority: cookies -> localStorage -> in memory 
-* if in localStorage, unable track users between subdomains
-* if in memory, then memory can't be shared between different tabs
-*/
- class MetadataStorage {
-  constructor({storageKey, disableCookies, domain, secure, sameSite, expirationDays}) {
+ * MetadataStorage involves SDK data persistance
+ * storage priority: cookies -> localStorage -> in memory
+ * if in localStorage, unable track users between subdomains
+ * if in memory, then memory can't be shared between different tabs
+ */
+class MetadataStorage {
+  constructor({
+    storageKey,
+    disableCookies,
+    domain,
+    secure,
+    sameSite,
+    expirationDays,
+  }) {
     this.storageKey = storageKey;
-    this.disableCookieStorage = !baseCookie.areCookiesEnabled() || disableCookies;
     this.domain = domain;
     this.secure = secure;
     this.sameSite = sameSite;
     this.expirationDays = expirationDays;
-    this.cookieDomain ='';
+
+    this.cookieDomain = '';
 
     if (!BUILD_COMPAT_REACT_NATIVE) {
       const writableTopDomain = topDomain(getLocation().href);
-      this.cookieDomain = domain || (writableTopDomain ? '.' + writableTopDomain : null);
+      this.cookieDomain =
+        domain || (writableTopDomain ? '.' + writableTopDomain : null);
     }
+
+    this.disableCookieStorage =
+      disableCookies ||
+      !baseCookie.areCookiesEnabled({
+        domain: this.cookieDomain,
+        secure: this.secure,
+        sameSite: this.sameSite,
+        expirationDays: this.expirationDays,
+      });
   }
 
   getCookieStorageKey() {
     if (!this.domain) {
       return this.storageKey;
     }
 
-    const suffix = this.domain.charAt(0) === '.' ? this.domain.substring(1) : this.domain;
+    const suffix =
+      this.domain.charAt(0) === '.' ? this.domain.substring(1) : this.domain;
 
     return `${this.storageKey}${suffix ? `_${suffix}` : ''}`;
   }
 
   /*
-  * Data is saved as delimited values rather than JSO to minimize cookie space
-  * Should not change order of the items
-  */
-  save({ deviceId, userId, optOut, sessionId, lastEventTime, eventId, identifyId, sequenceNumber }) {
+   * Data is saved as delimited values rather than JSO to minimize cookie space
+   * Should not change order of the items
+   */
+  save({
+    deviceId,
+    userId,
+    optOut,
+    sessionId,
+    lastEventTime,
+    eventId,
+    identifyId,
+    sequenceNumber,
+  }) {
     const value = [
       deviceId,
       Base64.encode(userId || ''), // used to convert not unicode to alphanumeric since cookies only use alphanumeric
@@ -54,22 +81,18 @@ import topDomain from './top-domain';
       lastEventTime ? lastEventTime.toString(32) : '0', // last time an event was set
       eventId ? eventId.toString(32) : '0',
       identifyId ? identifyId.toString(32) : '0',
-      sequenceNumber ? sequenceNumber.toString(32) : '0'
-    ].join('.'); 
+      sequenceNumber ? sequenceNumber.toString(32) : '0',
+    ].join('.');
 
     if (this.disableCookieStorage) {
       localStorage.setItem(this.storageKey, value);
     } else {
-      baseCookie.set(
-        this.getCookieStorageKey(),
-        value,
-        {
-          domain: this.cookieDomain,
-          secure: this.secure,
-          sameSite: this.sameSite,
-          expirationDays: this.expirationDays
-        }
-      );
+      baseCookie.set(this.getCookieStorageKey(), value, {
+        domain: this.cookieDomain,
+        secure: this.secure,
+        sameSite: this.sameSite,
+        expirationDays: this.expirationDays,
+      });
     }
   }
 
@@ -105,7 +128,7 @@ import topDomain from './top-domain';
       lastEventTime: parseInt(values[4], 32),
       eventId: parseInt(values[5], 32),
       identifyId: parseInt(values[6], 32),
-      sequenceNumber: parseInt(values[7], 32)
+      sequenceNumber: parseInt(values[7], 32),
     };
   }
 }

test/amplitude-client.js

@@ -391,7 +391,7 @@ describe('AmplitudeClient', function() {
         identifyId: 60,
         sequenceNumber: 70
       }
-      const storage = new MetadataStorage({storageKey: cookieName, disableCookieStorage: true});
+      const storage = new MetadataStorage({storageKey: cookieName, disableCookies: true});
       storage.save(cookieData);
 
       clock.tick(10);