feat: add PhStore.removeItem api and tests

src/brackets.js

@@ -406,8 +406,7 @@ define(function (require, exports, module) {
 
         // Load default languages and preferences
         Async.waitForAll([LanguageManager.ready, PreferencesManager.ready]).always(function () {
-            Promise.all([window._phoenixfsAppDirsCreatePromise, window.PhStore.storageReadyPromise])
-                .finally(_startupBrackets);
+            window._phoenixfsAppDirsCreatePromise.finally(_startupBrackets);
         });
     }
 

src/main.js

@@ -19,52 +19,6 @@
  *
  */
 
-/**
- * The bootstrapping module for brackets. This module sets up the require
- * configuration and loads the brackets module.
- */
-require.config({
-    paths: {
-        "text": "thirdparty/text/text",
-        "i18n": "thirdparty/i18n/i18n",
-
-        // The file system implementation. Change this value to use different
-        // implementations (e.g. cloud-based storage).
-        "fileSystemImpl": "filesystem/impls/appshell/AppshellFileSystem",
-        "preact-compat": "thirdparty/preact-compat/preact-compat.min",
-        "preact": "thirdparty/preact/preact"
-    },
-    map: {
-        "*": {
-            "thirdparty/CodeMirror2": "thirdparty/CodeMirror",
-            "thirdparty/preact": "preact-compat",
-            "view/PanelManager": "view/WorkspaceManager"  // For extension compatibility
-        }
-    },
-    waitSeconds: 60
-});
-
-if (window.location.search.indexOf("testEnvironment") > -1) {
-    require.config({
-        paths: {
-            "preferences/PreferencesImpl": "../test/TestPreferencesImpl"
-        },
-        locale: "en" // force English (US)
-    });
-} else {
-    /**
-     * hack for r.js optimization, move locale to another config call
-     *
-     * Use custom brackets property until CEF sets the correct navigator.language
-     * NOTE: When we change to navigator.language here, we also should change to
-     * navigator.language in ExtensionLoader (when making require contexts for each
-     * extension).
-     */
-    require.config({
-        locale: window.localStorage.getItem("locale") || window.navigator.language
-    });
-}
-
 /**
  * global util to convert jquery/js promise to a js promise. This can be used as an adapter when you do not know if the
  * promise in hand is a js or jquery deferred promise. This function will always return a normal js promise.
@@ -132,25 +86,73 @@ window.scriptObserver = new MutationObserver(callback);
 // Start observing the target node for configured mutations
 window.scriptObserver.observe(mainScripts, config);
 
-define(function (require) {
+window.PhStore.storageReadyPromise
+    .finally(()=>{
+        /**
+         * The bootstrapping module for brackets. This module sets up the require
+         * configuration and loads the brackets module.
+         */
+        require.config({
+            paths: {
+                "text": "thirdparty/text/text",
+                "i18n": "thirdparty/i18n/i18n",
 
+                // The file system implementation. Change this value to use different
+                // implementations (e.g. cloud-based storage).
+                "fileSystemImpl": "filesystem/impls/appshell/AppshellFileSystem",
+                "preact-compat": "thirdparty/preact-compat/preact-compat.min",
+                "preact": "thirdparty/preact/preact"
+            },
+            map: {
+                "*": {
+                    "thirdparty/CodeMirror2": "thirdparty/CodeMirror",
+                    "thirdparty/preact": "preact-compat",
+                    "view/PanelManager": "view/WorkspaceManager"  // For extension compatibility
+                }
+            },
+            waitSeconds: 60
+        });
 
-    // Load compatibility shims--these need to load early, be careful moving this
-    // Event dispatcher must be loaded before worker comm https://github.com/phcode-dev/phoenix/pull/678
-    require(["utils/Metrics", "utils/Compatibility", "utils/EventDispatcher"], function () {
-        window.Metrics = require("utils/Metrics");
-        // Load the brackets module. This is a self-running module that loads and runs the entire application.
-        try{
-            require(["brackets"]);
-        } catch (err) {
-            // try a cache refresh (not a full reset). this will happen in the service worker in the background
-            window.refreshServiceWorkerCache && window.refreshServiceWorkerCache();
-            // metrics api might not be available here as we were seeing no metrics raised. Only bugsnag there.
-            window.logger && window.logger.reportError(err,
-                'Critical error when loading brackets. Trying to reload again.');
-            // wait for 3 seconds for bugsnag to send report.
-            setTimeout(window.location.reload, 3000);
+        if (window.location.search.indexOf("testEnvironment") > -1) {
+            require.config({
+                paths: {
+                    "preferences/PreferencesImpl": "../test/TestPreferencesImpl"
+                },
+                locale: "en" // force English (US)
+            });
+        } else {
+            /**
+             * hack for r.js optimization, move locale to another config call
+             *
+             * Use custom brackets property until CEF sets the correct navigator.language
+             * NOTE: When we change to navigator.language here, we also should change to
+             * navigator.language in ExtensionLoader (when making require contexts for each
+             * extension).
+             */
+            require.config({
+                locale: window.PhStore.getItem("locale") || window.navigator.language
+            });
         }
-    });
-});
 
+        define(function (require) {
+
+
+            // Load compatibility shims--these need to load early, be careful moving this
+            // Event dispatcher must be loaded before worker comm https://github.com/phcode-dev/phoenix/pull/678
+            require(["utils/Metrics", "utils/Compatibility", "utils/EventDispatcher"], function () {
+                window.Metrics = require("utils/Metrics");
+                // Load the brackets module. This is a self-running module that loads and runs the entire application.
+                try{
+                    require(["brackets"]);
+                } catch (err) {
+                    // try a cache refresh (not a full reset). this will happen in the service worker in the background
+                    window.refreshServiceWorkerCache && window.refreshServiceWorkerCache();
+                    // metrics api might not be available here as we were seeing no metrics raised. Only bugsnag there.
+                    window.logger && window.logger.reportError(err,
+                        'Critical error when loading brackets. Trying to reload again.');
+                    // wait for 3 seconds for bugsnag to send report.
+                    setTimeout(window.location.reload, 3000);
+                }
+            });
+        });
+    });

src/storage.js

@@ -130,7 +130,7 @@
      * Retrieves the value associated with the specified key from the browser's local storage.
      *
      * @param {string} key - The key to retrieve the value for.
-     * @returns {object|null} - The value associated with the specified key. Returns null if the key does not exist.
+     * @returns {string|number|boolean|object|null} - The value associated with the specified key. Returns null if the key does not exist.
      */
     function getItem(key) {
         let cachedResult = cache[key];
@@ -162,7 +162,7 @@
      * Sets the value of a specified key in the localStorage.
      *
      * @param {string} key - The key to set the value for.
-     * @param {*} value - The value to be stored. Can be any valid JSON serializable data type.
+     * @param {string|number|boolean|object} value - The value to be stored. Can be any valid JSON serializable data type.
      *
      */
     function setItem(key, value) {
@@ -190,6 +190,16 @@
         PhStore.trigger(key, CHANGE_TYPE_INTERNAL);
     }
 
+    /**
+     * Removes an item from storage. This will trigger a change notification on removal if watchers are attached.
+     * Watchers are unaffected on removal, you will still get notifications if the key gets created in the future.
+     *
+     * @param {string} key - The key to remove
+     */
+    function removeItem(key) {
+        setItem(key, null);
+    }
+
     /**
      * Enables best effort monitoring of external changes to the specified key when there are multiple Phoenix
      * windows/instances. By default, PhStore.on(<key name>, fn) only triggers for key value changes within
@@ -263,6 +273,7 @@
     const PhStore = {
         getItem,
         setItem,
+        removeItem,
         flushDB,
         watchExternalChanges,
         unwatchExternalChanges,

src/utils/Global.js

@@ -67,19 +67,19 @@ define(function (require, exports, module) {
 
     // Locale-related APIs
     global.brackets.isLocaleDefault = function () {
-        return !global.localStorage.getItem("locale");
+        return !global.PhStore.getItem("locale");
     };
 
     global.brackets.getLocale = function () {
         // By default use the locale that was determined in brackets.js
-        return params.get("testEnvironment") ? "en" : (global.localStorage.getItem("locale") || global.require.s.contexts._.config.locale);
+        return params.get("testEnvironment") ? "en" : (global.PhStore.getItem("locale") || global.require.s.contexts._.config.locale);
     };
 
     global.brackets.setLocale = function (locale) {
         if (locale) {
-            global.localStorage.setItem("locale", locale);
+            global.PhStore.setItem("locale", locale);
         } else {
-            global.localStorage.removeItem("locale");
+            global.PhStore.removeItem("locale");
         }
     };
 

src/utils/Metrics.js

@@ -47,13 +47,13 @@ define(function (require, exports, module) {
 
     let isFirstUseDay;
     function _setFirstDayFlag() {
-        let firstUseDay= localStorage.getItem("healthData.firstUseDay");
-        if(!firstUseDay){
-            firstUseDay = new Date();
-            localStorage.setItem("healthData.firstUseDay", `${firstUseDay.getTime()}`);
-        } else {
-            firstUseDay = new Date(parseInt(firstUseDay));
+        const firstUseDayKey = "healthData.firstUseDay";
+        let firstBootTime = window.PhStore.getItem(firstUseDayKey);
+        if(!firstBootTime){
+            firstBootTime = Date.now();
+            window.PhStore.setItem(firstUseDayKey, firstBootTime);
         }
+        let firstUseDay= new Date(firstBootTime);
         let dayAfterFirstUse = new Date(firstUseDay);
         dayAfterFirstUse.setUTCDate(firstUseDay.getUTCDate() + 1);
         let today = new Date();

test/spec/Storage-integ-test.js

@@ -71,12 +71,21 @@ define(function (require, exports, module) {
         it("Should be able to get and set different value types", async function () {
             expectSetGetSuccess(1);
             expectSetGetSuccess("");
+            expectSetGetSuccess(null);
             expectSetGetSuccess(0);
             expectSetGetSuccess("hello");
             expectSetGetSuccess({hello: {message: "world"}});
             expectSetGetSuccess([1, "3"]);
         });
 
+        it("Should be able to remove item", async function () {
+            const value = "hello";
+            PhStore.setItem(testKey, value);
+            expect(PhStore.getItem(testKey)).toEql(value);
+            PhStore.removeItem(testKey);
+            expect(PhStore.getItem(testKey)).toEql(null);
+        });
+
         it("Should be able to get and set with lmdb node connector in tauri", async function () {
             if(!Phoenix.browser.isTauri){
                 return;
@@ -181,7 +190,7 @@ define(function (require, exports, module) {
             });
 
             const newValue = "hello";
-            await awaits(500);// let time pass as the lowest resolution for time check in browser is 1 ms
+            await awaits(500);// let time pass
             testWindow.PhStore.setItem(testKey, newValue); // set in phoenix, it should eventually come to this window
             expect(testWindow.PhStore.getItem(testKey)).toEql(newValue);
             await awaitsFor(function () {
@@ -191,5 +200,33 @@ define(function (require, exports, module) {
             expect(changedValue).toEql(newValue);
         });
 
+        it("Should get changed notification in this window, if removing watched item in external window", async function () {
+            const currentWinVal = "externalRemove";
+            PhStore.watchExternalChanges(testKey);
+            testWindow.PhStore.watchExternalChanges(testKey);
+
+            PhStore.setItem(testKey, currentWinVal);
+            expect(PhStore.getItem(testKey)).toEql(currentWinVal);
+            await awaitsFor(function () {
+                return testWindow.PhStore.getItem(testKey) === currentWinVal;
+            });
+
+            // now both window has set the value. remove it and see if we get notifications
+
+            let changeType, changedValue = undefined;
+            PhStore.on(testKey, (_event, type)=>{
+                changeType = type;
+                changedValue = PhStore.getItem(testKey); // the new key should be updated when you get the event
+            });
+
+            testWindow.PhStore.removeItem(testKey); // remove in phoenix, it should eventually come to this window
+            expect(testWindow.PhStore.getItem(testKey)).toEql(null);
+            await awaitsFor(function () {
+                return changedValue === null;
+            });
+            expect(changeType).toEql("External");
+            expect(changedValue).toEql(null);
+        });
+
     });
 });