Merge pull request #70 from simonsobs/dev

Fix a bug where `provide()` might be called outside of `setup()`

.eslintrc.ts

@@ -1,7 +1,7 @@
 module.exports = {
   root: true,
   env: {
-    es2021: true,
+    es2022: true,
   },
   extends: [
     "plugin:vue/essential",
@@ -10,7 +10,7 @@ module.exports = {
     "plugin:prettier/recommended",
   ],
   parserOptions: {
-    ecmaVersion: 2020,
+    ecmaVersion: 2022,
   },
   rules: {
     "no-console": import.meta.env.NODE_ENV === "production" ? "warn" : "off",

src/app/set-title.ts

@@ -1,12 +1,12 @@
-import { computed, toValue } from "vue";
-import { useTitle } from "@vueuse/core";
 import { useConfig } from "@/utils/config";
+import { useTitle } from "@vueuse/core";
+import { computed } from "vue";
 
 function useSetTitle() {
   const { config } = useConfig();
   const title = computed(() => {
-    const appName = toValue(config).appName;
-    const apiName = toValue(config).apiName;
+    const appName = config.value.appName;
+    const apiName = config.value.apiName;
     return `${appName}: ${apiName}`;
   });
 

src/graphql/urql/client/provide.ts

@@ -1,7 +1,7 @@
-import { ref, unref, toRefs, watchEffect } from "vue";
-import type { Ref, MaybeRef } from "vue";
-import { provideClient } from "@urql/vue";
 import type { Client } from "@urql/vue";
+import { provideClient } from "@urql/vue";
+import type { MaybeRef, Ref } from "vue";
+import { computed, ref, unref, watchEffect } from "vue";
 
 import { useConfig } from "@/utils/config";
 
@@ -15,7 +15,7 @@ export function useProvideClient() {
 
 function useUrl() {
   const { config } = useConfig();
-  const { apiHttp: url } = toRefs(config.value);
+  const url = computed(() => config.value.apiHttp);
   return url;
 }
 

src/utils/color-theme/color-theme.ts

@@ -1,21 +1,21 @@
-import { ref } from "vue";
 import type { MaybeRef } from "vue";
+import { computed, unref } from "vue";
 
 import { useDynamicColors } from "@/utils/dynamic-color";
 import {
-  useDynamicColorsOnMonacoEditor,
   useDarkModeOnMonacoEditor,
+  useDynamicColorsOnMonacoEditor,
 } from "./monaco-editor";
-import { useDynamicColorsOnVuetify, useDarkModeOnVuetify } from "./vuetify";
+import { useDarkModeOnVuetify, useDynamicColorsOnVuetify } from "./vuetify";
 
 const DEFAULT_SOURCE_COLOR_HEX = "#607D8B"; // blue grey
 // const DEFAULT_SOURCE_COLOR_HEX = "#E91E63"; // pink
 
-export function useColorTheme(sourceColorHex?: MaybeRef<string>) {
-  sourceColorHex = ref(sourceColorHex ?? DEFAULT_SOURCE_COLOR_HEX);
+export function useColorTheme(sourceColorHex?: MaybeRef<string | undefined>) {
+  const source = computed(() => unref(sourceColorHex) || DEFAULT_SOURCE_COLOR_HEX);
 
-  const optionsLight = { sourceColorHex, dark: false };
-  const optionsDark = { sourceColorHex, dark: true };
+  const optionsLight = { sourceColorHex: source, dark: false };
+  const optionsDark = { sourceColorHex: source, dark: true };
 
   const { colors: lightColors } = useDynamicColors(optionsLight);
   const { colors: darkColors } = useDynamicColors(optionsDark);

src/utils/config/ProvideConfig.vue

@@ -15,8 +15,10 @@
 /**
  * Load config asynchronously and provide it to the child components.
  */
+import { until } from "@vueuse/core";
 import { useLoadConfig } from "@/utils/config";
 import { useProvideConfig } from "@/utils/config";
 const { error, config } = await useLoadConfig();
+await until(config).not.toBeNull();
 useProvideConfig(config);
 </script>

src/utils/config/__tests__/provide-config.spec.ts

@@ -0,0 +1,139 @@
+import { describe, expect, it } from "vitest";
+import type { App, Ref } from "vue";
+import { createApp, defineComponent, inject, nextTick, shallowRef, watch } from "vue";
+
+import { injectionKey } from "../key";
+import { useProvideConfigT } from "../provide-config";
+
+type SetupFunction = () => Record<string, unknown>;
+
+interface WitSetupsOptions {
+  setupParent: SetupFunction;
+  setupChild: SetupFunction;
+}
+
+class DisposableApp implements Disposable {
+  constructor(private app: App) {}
+
+  [Symbol.dispose]() {
+    this.app.unmount();
+  }
+}
+
+function withSetups(options: WitSetupsOptions): DisposableApp {
+  const ParentComponent = defineComponent({
+    setup: options.setupParent,
+    template: "<child-component />",
+  });
+
+  const ChildComponent = defineComponent({
+    setup: options.setupChild,
+    template: "<template />",
+  });
+
+  const app = createApp(ParentComponent);
+  app.component("ChildComponent", ChildComponent);
+  app.mount(document.createElement("div"));
+  return new DisposableApp(app);
+}
+
+describe("useProvideConfigT", () => {
+  it("provides and updates config reactively when given a ShallowRef", async () => {
+    const config = shallowRef({ key: "initial" });
+    let injectedConfig: Ref<{ key: string }> | undefined;
+
+    using _ = withSetups({
+      setupParent() {
+        useProvideConfigT(config);
+        return {};
+      },
+      setupChild() {
+        injectedConfig = inject(injectionKey);
+        return {};
+      },
+    });
+
+    await nextTick();
+    expect(injectedConfig?.value).toEqual({ key: "initial" });
+
+    config.value = { key: "updated" };
+    await nextTick();
+    expect(injectedConfig?.value).toEqual({ key: "updated" });
+
+    config.value = { key: "final" };
+    await nextTick();
+    expect(injectedConfig?.value).toEqual({ key: "final" });
+  });
+
+  it("works with non-ref values", async () => {
+    let injectedConfig: Ref<{ key: string }> | undefined;
+
+    using _ = withSetups({
+      setupParent() {
+        useProvideConfigT({ key: "static" });
+        return {};
+      },
+      setupChild() {
+        injectedConfig = inject(injectionKey);
+        return {};
+      },
+    });
+
+    await nextTick();
+    expect(injectedConfig?.value).toEqual({ key: "static" });
+  });
+
+  it("returns the provided ShallowRef", () => {
+    let returnedConfig: Ref<{ key: string }> | undefined;
+
+    using _ = withSetups({
+      setupParent() {
+        const result = useProvideConfigT({ key: "test" });
+        returnedConfig = result.config;
+        return {};
+      },
+      setupChild() {
+        return {};
+      },
+    });
+
+    expect(returnedConfig?.value).toEqual({ key: "test" });
+  });
+
+  it("maintains shallow reactivity", async () => {
+    const config = shallowRef({ nested: { value: "initial" } });
+    let injectedConfig: Ref<{ nested: { value: string } }> | undefined;
+    let watchCount = 0;
+
+    using _ = withSetups({
+      setupParent() {
+        useProvideConfigT(config);
+        return {};
+      },
+      setupChild() {
+        injectedConfig = inject(injectionKey);
+        watch(
+          () => injectedConfig?.value,
+          () => {
+            watchCount++;
+          },
+          { deep: true }
+        );
+        return {};
+      },
+    });
+
+    await nextTick();
+    expect(injectedConfig?.value.nested.value).toBe("initial");
+
+    config.value.nested.value = "updated";
+    await nextTick();
+    expect(injectedConfig?.value.nested.value).toBe("updated");
+    expect(watchCount).toBe(0); // Should not trigger watch due to shallow reactivity
+
+    config.value = { nested: { value: "new object" } };
+    await nextTick();
+    expect(injectedConfig?.value.nested.value).toBe("new object");
+    expect(watchCount).toBe(1); // Should trigger watch
+  });
+});

src/utils/config/index.ts

@@ -1,11 +1,11 @@
-import type { MaybeRefOrGetter } from "vue";
+import type { ShallowRef } from "vue";
 
-import { defaultConfig, validateConfig } from "./config";
 import type { Config } from "./config";
+import { defaultConfig, validateConfig } from "./config";
 import { useConfigT } from "./inject-config";
 import { useLoadConfigT } from "./load-config";
-import { useProvideConfigT } from "./provide-config";
 import { useOverrideT } from "./override";
+import { useProvideConfigT } from "./provide-config";
 
 export type { Config };
 
@@ -19,9 +19,8 @@ export const useConfig = () => useConfigT<Config>();
  * Provide the config to the child components.
  * In the child components, use `useConfig` to get the config.
  */
-export const useProvideConfig = (
-  config: Exclude<MaybeRefOrGetter<Config | null>, null>
-) => useProvideConfigT<Config>(config);
+export const useProvideConfig = (config: Config | ShallowRef<Config>) =>
+  useProvideConfigT<Config>(config);
 
 /**
  * Read the config from the config file.

src/utils/config/provide-config.ts

@@ -1,37 +1,9 @@
-import { ref, provide, watchEffect, toValue } from "vue";
-import type { MaybeRefOrGetter, InjectionKey, Ref } from "vue";
-
+import type { InjectionKey, Ref, ShallowRef } from "vue";
+import { isRef, provide, shallowRef } from "vue";
 import { injectionKey } from "./key";
-/**
- * Provides a configuration of type `T` to child components.
- *
- * This function creates a reactive reference to the config and provides it
- * to child components using Vue's provide/inject system.
- *
- * @template T The type of the configuration object
- * @param The configuration, which can be:
- *        - A value of type T
- *        - A Ref<T | null>
- *        - A getter function returning T or null
- *
- * @remarks
- * In child components, use `useConfig` to retrieve this provided config.
- * The config will only be provided once it has a non-null value.
- * Subsequent updates to the config (including to undefined) will be reflected
- * in the provided reference, but will not trigger a new provide.
- */
-export function useProvideConfigT<T>(
-  config: Exclude<MaybeRefOrGetter<T | null>, null>
-) {
-  const configRef = ref<T>() as Ref<T>;
-  let provided = false;
 
-  watchEffect(() => {
-    const value = toValue(config);
-    if (value === null) return;
-    configRef.value = value;
-    if (provided) return;
-    provide(injectionKey as InjectionKey<Ref<T>>, configRef);
-    provided = true;
-  });
+export function useProvideConfigT<T>(config: T | ShallowRef<T>) {
+  const configRef = isRef(config) ? config : shallowRef(config);
+  provide(injectionKey as InjectionKey<Ref<T>>, configRef);
+  return { config: configRef };
 }

src/utils/dynamic-color/__tests__/dynamic-scheme.spec.ts

@@ -110,4 +110,20 @@ describe("useDynamicScheme", () => {
     sourceColorHex.value = "#2196F3";
     expect(sourceColorHct.value.toInt()).toBe(argbFromHex("#2196F3"));
   });
+
+  it("use default hex on error", () => {
+    const options: UseDynamicSchemeOptions = {
+      sourceColorHex: "not-a-hex",
+    };
+    const { sourceColorHex, sourceColorHct } = useDynamicScheme(options);
+    expect(sourceColorHex.value).toBe("#6750A4");
+    expect(sourceColorHct.value.toInt()).toBe(argbFromHex("#6750A4"));
+
+    sourceColorHex.value = "#2196F3";
+    expect(sourceColorHct.value.toInt()).toBe(argbFromHex("#2196F3"));
+
+    sourceColorHex.value = "not-a-hex";
+    expect(sourceColorHex.value).toBe("#6750A4");
+    expect(sourceColorHct.value.toInt()).toBe(argbFromHex("#6750A4"));
+  });
 });