Restrict interfaces with projects for components

packages/components/src/components/SquiggleChart.tsx

@@ -11,15 +11,13 @@ import { useSquiggle } from "../lib/hooks";
 import { SquiggleViewer } from "./SquiggleViewer";
 import { JsImports } from "../lib/jsImports";
 
-export interface SquiggleChartProps {
+export type SquiggleChartProps = {
   /** The input string for squiggle */
-  code?: string;
+  code: string;
   /** Allows to re-run the code if code hasn't changed */
   executionId?: number;
   /** If the output requires monte carlo sampling, the amount of samples */
   sampleCount?: number;
-  /** The amount of points returned to draw the distribution */
-  environment?: environment;
   /** If the result is a function, where the function domain starts */
   diagramStart?: number;
   /** If the result is a function, where the function domain ends */
@@ -54,22 +52,34 @@ export interface SquiggleChartProps {
   /** Whether to show vega actions to the user, so they can copy the chart spec */
   distributionChartActions?: boolean;
   enableLocalSettings?: boolean;
-  /** The project that this execution is part of */
+} & (StandaloneExecutionProps | ProjectExecutionProps);
-  project?: SqProject;
-  /** The name of the squiggle execution source. Generates a UUID if not given */
-  sourceName?: string;
-  /** The sources that this execution continues */
-  includes?: string[];
-}
 
+// Props needed for a standalone execution
+type StandaloneExecutionProps = {
+  /** Project must be undefined */
+  project?: undefined;
+  /** Includes must be undefined */
+  includes?: undefined;
+  /** The amount of points returned to draw the distribution, not needed if using a project */
+  environment?: environment;
+};
+
+// Props needed when executing inside a project.
+type ProjectExecutionProps = {
+  /** environment must be undefined (we don't set it here, users can set the environment outside the execution) */
+  environment?: undefined;
+  /** The project that this execution is part of */
+  project: SqProject;
+  /** What other squiggle sources from the project to include. Default none */
+  includes?: string[];
+};
 const defaultOnChange = () => {};
 const defaultImports: JsImports = {};
 
 export const SquiggleChart: React.FC<SquiggleChartProps> = React.memo(
-  ({
+  (props: SquiggleChartProps) => {
-    code,
+    const {
       executionId = 0,
-    environment,
       onChange = defaultOnChange, // defaultOnChange must be constant, don't move its definition here
       height = 200,
       jsImports = defaultImports,
@@ -88,16 +98,20 @@ export const SquiggleChart: React.FC<SquiggleChartProps> = React.memo(
       xAxisType = "number",
       distributionChartActions,
       enableLocalSettings = false,
-    sourceName,
-    includes = [],
       project = SqProject.create(),
-  }) => {
-    const { result, bindings } = useSquiggle({
-      sourceName,
-      includes,
-      project,
       code,
-      environment,
+      includes = [],
+    } = props;
+
+    const p = project ?? SqProject.create();
+    if (!project && props.environment) {
+      p.setEnvironment(props.environment);
+    }
+
+    const { result, bindings } = useSquiggle({
+      includes,
+      project: p,
+      code,
       jsImports,
       onChange,
       executionId,
@@ -133,7 +147,7 @@ export const SquiggleChart: React.FC<SquiggleChartProps> = React.memo(
         height={height}
         distributionPlotSettings={distributionPlotSettings}
         chartSettings={chartSettings}
-        environment={environment ?? defaultEnvironment}
+        environment={p.getEnvironment()}
         enableLocalSettings={enableLocalSettings}
       />
     );

packages/components/src/lib/hooks/useSquiggle.ts

@@ -7,9 +7,7 @@ type SquiggleArgs = {
   code?: string;
   executionId?: number;
   jsImports?: JsImports;
-  environment?: environment;
   project: SqProject;
-  sourceName?: string;
   includes: string[];
   onChange?: (expr: SqValue | undefined, sourceName: string) => void;
 };
@@ -17,32 +15,15 @@ type SquiggleArgs = {
 const importSourceName = (sourceName: string) => "imports-" + sourceName;
 
 export const useSquiggle = (args: SquiggleArgs) => {
-  const autogenName = useMemo(() => uuid.v4(), []);
+  const sourceName = useMemo(() => uuid.v4(), []);
 
   const result = useMemo(
     () => {
       const project = args.project;
       let needsClean = true;
 
-      let sourceName = "";
-      // If the user specified a source and it already exists, assume we don't
-      // own the source
-      if (args.sourceName && project.getSource(args.sourceName)) {
-        needsClean = false;
-        sourceName = args.sourceName;
-      } else {
-        // Otherwise create a source, either with the name given or an automatic one
-        if (args.sourceName) {
-          sourceName = args.sourceName;
-        } else {
-          sourceName = autogenName;
-        }
       project.setSource(sourceName, args.code ?? "");
-      }
       let includes = args.includes;
-      if (args.environment) {
-        project.setEnvironment(args.environment);
-      }
       if (args.jsImports && Object.keys(args.jsImports).length) {
         const importsSource = jsImportsToSquiggleCode(args.jsImports);
         project.setSource(importSourceName(sourceName), importsSource);
@@ -54,8 +35,18 @@ export const useSquiggle = (args: SquiggleArgs) => {
       const bindings = project.getBindings(sourceName);
       return { result, bindings, sourceName, needsClean };
     },
+    // This complains about executionId not being used inside the function body.
+    // This is on purpose, as executionId simply allows you to run the squiggle
+    // code again
     // eslint-disable-next-line react-hooks/exhaustive-deps
-    [args.code, args.environment, args.jsImports, args.executionId]
+    [
+      args.code,
+      args.jsImports,
+      args.executionId,
+      sourceName,
+      args.includes,
+      args.project,
+    ]
   );
 
   const { onChange } = args;
@@ -67,17 +58,13 @@ export const useSquiggle = (args: SquiggleArgs) => {
     );
   }, [result, onChange]);
 
-  useEffect(
+  useEffect(() => {
-    () => {
     return () => {
-        if (result.needsClean) args.project.removeSource(result.sourceName);
+      args.project.removeSource(result.sourceName);
       if (args.project.getSource(importSourceName(result.sourceName)))
         args.project.removeSource(result.sourceName);
     };
-    },
+  }, [args.project, result.sourceName]);
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-    []
-  );
 
   return result;
 };

packages/components/test/cleanup.test.tsx

@@ -10,6 +10,7 @@ test("Creates and cleans up source with no name", async () => {
   const { unmount } = render(
     <SquiggleChart code={"normal(0, 1)"} project={project} />
   );
+
   expect(project.getSourceIds().length).toBe(1);
 
   const sourceId = project.getSourceIds()[0];
@@ -19,43 +20,3 @@ test("Creates and cleans up source with no name", async () => {
   expect(project.getSourceIds().length).toBe(0);
   expect(project.getSource(sourceId)).toBe(undefined);
 });
-
-test("Does not clean up existing source", async () => {
-  const project = SqProject.create();
-
-  project.setSource("main", "normal(0, 1)");
-
-  const { unmount } = render(
-    <SquiggleChart sourceName={"main"} project={project} />
-  );
-
-  expect(project.getSourceIds()).toStrictEqual(["main"]);
-
-  const sourceId = project.getSourceIds()[0];
-  expect(project.getSource(sourceId)).toBe("normal(0, 1)");
-
-  unmount();
-  expect(project.getSourceIds()).toStrictEqual(["main"]);
-  expect(project.getSource(sourceId)).toBe("normal(0, 1)");
-});
-
-test("Does clean up when given non-existant source", async () => {
-  const project = SqProject.create();
-
-  const { unmount } = render(
-    <SquiggleChart
-      code={"normal(0, 1)"}
-      sourceName={"main"}
-      project={project}
-    />
-  );
-
-  expect(project.getSourceIds()).toStrictEqual(["main"]);
-
-  const sourceId = project.getSourceIds()[0];
-  expect(project.getSource(sourceId)).toBe("normal(0, 1)");
-
-  unmount();
-  expect(project.getSourceIds()).toStrictEqual([]);
-  expect(project.getSource(sourceId)).toBe(undefined);
-});