PR feedback

lib/papi-dts/papi.d.ts

@@ -3469,10 +3469,9 @@ declare module 'renderer/hooks/papi-hooks/use-promise.hook' {
   /**
    * Awaits a promise and returns a loading value while the promise is unresolved
    *
-   * @param promiseFactoryCallback A function that returns the promise to await. If the promise
-   *   resolves to undefined, the value will not change. If this callback is undefined, the current
-   *   value will be returned (defaultValue unless it was previously changed and preserveValue is
-   *   true), and there will be no loading.
+   * @param promiseFactoryCallback A function that returns the promise to await. If this callback is
+   *   undefined, the current value will be returned (defaultValue unless it was previously changed
+   *   and preserveValue is true), and there will be no loading.
    *
    *   WARNING: MUST BE STABLE - const or wrapped in useCallback. The reference must not be updated
    *   every render
@@ -3490,7 +3489,7 @@ declare module 'renderer/hooks/papi-hooks/use-promise.hook' {
    *   - `isLoading`: whether the promise is waiting to be resolved
    */
   const usePromise: <T>(
-    promiseFactoryCallback: (() => Promise<T | undefined>) | undefined,
+    promiseFactoryCallback: (() => Promise<T>) | undefined,
     defaultValue: T,
     preserveValue?: boolean,
   ) => [value: T, isLoading: boolean];

src/extension-host/services/extension.service.ts

@@ -759,7 +759,7 @@ async function reloadExtensions(shouldDeactivateExtensions: boolean): Promise<vo
     }
 
   // Save extensions that have JavaScript to run
-  // If main is am empty string, having no JavaScript is intentional. Do not load this extension
+  // If main is an empty string, having no JavaScript is intentional. Do not load this extension
   availableExtensions = allExtensions.filter((extension) => extension.main);
 
   // Store their base URIs in the extension storage service

src/renderer/components/docking/platform-dock-layout.component.tsx

@@ -583,9 +583,9 @@ function updateWebViewDefinition(
 // #endregion
 
 export default function PlatformDockLayout() {
-  // This ref will always be defined, and `undefined` doesn't work with `useRef` the same as `null`
-  // eslint-disable-next-line no-type-assertion/no-type-assertion, no-null/no-null
-  const dockLayoutRef = useRef<DockLayout>(null!);
+  // This ref will always be defined
+  // eslint-disable-next-line no-type-assertion/no-type-assertion
+  const dockLayoutRef = useRef<DockLayout>(undefined!);
 
   /**
    * OnLayoutChange function from `web-view.service.ts` once this docklayout is registered.

src/renderer/context/papi-context/test.context.ts

@@ -1,4 +1,4 @@
 import { createContext } from 'react';
 
-const TestContext = createContext<string>('');
+const TestContext = createContext('');
 export default TestContext;

src/renderer/hooks/papi-hooks/use-promise.hook.ts

@@ -3,10 +3,9 @@ import { useEffect, useState } from 'react';
 /**
  * Awaits a promise and returns a loading value while the promise is unresolved
  *
- * @param promiseFactoryCallback A function that returns the promise to await. If the promise
- *   resolves to undefined, the value will not change. If this callback is undefined, the current
- *   value will be returned (defaultValue unless it was previously changed and preserveValue is
- *   true), and there will be no loading.
+ * @param promiseFactoryCallback A function that returns the promise to await. If this callback is
+ *   undefined, the current value will be returned (defaultValue unless it was previously changed
+ *   and preserveValue is true), and there will be no loading.
  *
  *   WARNING: MUST BE STABLE - const or wrapped in useCallback. The reference must not be updated
  *   every render
@@ -24,7 +23,7 @@ import { useEffect, useState } from 'react';
  *   - `isLoading`: whether the promise is waiting to be resolved
  */
 const usePromise = <T>(
-  promiseFactoryCallback: (() => Promise<T | undefined>) | undefined,
+  promiseFactoryCallback: (() => Promise<T>) | undefined,
   defaultValue: T,
   preserveValue = true,
 ): [value: T, isLoading: boolean] => {
@@ -40,8 +39,7 @@ const usePromise = <T>(
         const result = await promiseFactoryCallback();
         // If the promise was not already replaced, update the value
         if (promiseIsCurrent) {
-          // If the promise returned undefined, it purposely did this to do nothing. Maybe its dependencies are not set up
-          if (result !== undefined) setValue(() => result);
+          setValue(() => result);
           setIsLoading(false);
         }
       }

src/shared/services/settings.service.ts

@@ -5,7 +5,7 @@ import { SettingNames, SettingTypes } from 'papi-shared-types';
 /** All message subscriptions - emitters that emit an event each time a setting is updated */
 const onDidUpdateSettingEmitters = new Map<
   SettingNames,
-  PapiEventEmitter<SettingTypes | undefined>
+  PapiEventEmitter<SettingTypes[SettingNames] | undefined>
 >();
 
 /**
@@ -68,7 +68,7 @@ const subscribeToSetting = <SettingName extends SettingNames>(
       key,
       // Assert type of the general SettingTypes of the emitter.
       // eslint-disable-next-line no-type-assertion/no-type-assertion
-      emitter as unknown as PapiEventEmitter<SettingTypes | undefined>,
+      emitter as PapiEventEmitter<SettingTypes[SettingNames] | undefined>,
     );
   }
   return emitter.subscribe(callback);

src/shared/utils/papi-util.test.ts

@@ -19,7 +19,7 @@ class Stuff {
   }
 }
 
-describe('PAPI Utils', () => {
+describe('PAPI Util Functions: serializeRequestType and deserializeRequestType', () => {
   it('can serialize and deserialize request types', () => {
     const CATEGORY = 'myCategory';
     const DIRECTIVE = 'myDirective';
@@ -72,7 +72,7 @@ describe('PAPI Utils', () => {
   });
 });
 
-describe('deepEqual', () => {
+describe('PAPI Util Function: deepEqual', () => {
   it('is true for empty objects', () => {
     expect(deepEqual({}, {})).toBeTruthy();
   });
@@ -207,8 +207,8 @@ describe('deepEqual', () => {
   });
 });
 
-describe('serialize and deserialize', () => {
-  it('handles values without null or undefined the same as JSON.stringify/parse', () => {
+describe('PAPI Util Functions: serialize and deserialize', () => {
+  it('handle values without null or undefined the same as JSON.stringify/parse', () => {
     const testObject = { foo: 'fooValue', bar: 3, baz: { bazInternal: 'LOL' } };
     const serializedTestObject = JSON.stringify(testObject);
     expect(serialize(testObject)).toEqual(JSON.stringify(testObject));
@@ -222,21 +222,20 @@ describe('serialize and deserialize', () => {
     expect(serialize([3, 5, 7])).toEqual(JSON.stringify([3, 5, 7]));
     expect(deepEqual(deserialize('[3,5,7]'), JSON.parse('[3,5,7]'))).toBeTruthy();
   });
-  it('changes null and undefined to a moniker and removes them upon deserialization', () => {
-    const moniker = '__NIL__';
+  it('exclusively use null in JSON strings and exclusively uses undefined in JS objects', () => {
     const testObject = { foo: 'fooValue', bar: undefined, baz: null };
     expect(serialize(testObject)).toEqual(
-      JSON.stringify({ foo: 'fooValue', bar: moniker, baz: moniker }),
+      JSON.stringify({ foo: 'fooValue', bar: null, baz: null }),
     );
     expect(deepEqual({ foo: 'fooValue' }, deserialize(serialize(testObject)))).toBeTruthy();
     expect(deepEqual({ foo: 'fooValue' }, deserialize(JSON.stringify(testObject)))).toBeTruthy();
   });
-  it('handles deeply nested null/undefined values', () => {
+  it('handle deeply nested null/undefined values', () => {
     const deepNesting = { a: { b: { c: { d: { e: 'something', undef: undefined, nil: null } } } } };
     const roundTrip = { a: { b: { c: { d: { e: 'something' } } } } };
     expect(deepEqual(roundTrip, deserialize(serialize(deepNesting)))).toBeTruthy();
   });
-  it('works with custom replacers/revivers', () => {
+  it('work with custom replacers/revivers', () => {
     const testObject = { a: 5 };
     const replacer = (_key: string, value: unknown) => {
       if (value === 5) return 10;
@@ -253,7 +252,7 @@ describe('serialize and deserialize', () => {
     ).toBeTruthy();
     expect(deserialize(serialize({ lazarus: undefined }), reviver).lazarus).toEqual('resurrected');
   });
-  it('turns null values in an array into undefined when deserializing', () => {
+  it('turn null values in an array into undefined when deserializing', () => {
     // Type asserting after deserializing
     // eslint-disable-next-line no-type-assertion/no-type-assertion, @typescript-eslint/no-explicit-any
     const transformedArray = deserialize(serialize([1, undefined, null, 4])) as Array<any>;
@@ -264,7 +263,7 @@ describe('serialize and deserialize', () => {
   });
 });
 
-describe('isSerializable', () => {
+describe('PAPI Util Function: isSerializable', () => {
   it('successfully determines empty object is serializable', () => {
     const objectToSerialize = {};
     expect(isSerializable(objectToSerialize)).toBeTruthy();

src/shared/utils/papi-util.ts

@@ -158,9 +158,6 @@ export function deepEqual(a: unknown, b: unknown) {
 
 // #region Serialization, deserialization, encoding, and decoding functions
 
-// Something to stand for both "undefined" and "null"
-const NIL_MONIKER: string = '__NIL__';
-
 /**
  * Converts a JavaScript value to a JSON string, changing `null` and `undefined` values to a moniker
  * that deserializes to `undefined`.
@@ -189,9 +186,9 @@ export function serialize(
   const undefinedReplacer = (replacerKey: string, replacerValue: unknown) => {
     let newValue = replacerValue;
     if (replacer) newValue = replacer(replacerKey, newValue);
-    // If a "null" slips into the data somehow, we need to deal with it
+    // All `undefined` values become `null` on the way from JS objects into JSON strings
     // eslint-disable-next-line no-null/no-null
-    if (newValue === undefined || newValue === null) newValue = NIL_MONIKER;
+    if (newValue === undefined) newValue = null;
     return newValue;
   };
   return JSON.stringify(value, undefinedReplacer, space);
@@ -222,9 +219,9 @@ export function deserialize(
 ): any {
   const undefinedReviver = (replacerKey: string, replacerValue: unknown) => {
     let newValue = replacerValue;
-    // If someone passes through a value with "null", we need to handle it
+    // All `null` values become `undefined` on the way from JSON strings into JS objects
     // eslint-disable-next-line no-null/no-null
-    if (newValue === NIL_MONIKER || newValue === null) newValue = undefined;
+    if (newValue === null) newValue = undefined;
     if (reviver) newValue = reviver(replacerKey, newValue);
     return newValue;
   };