Prepare Relay conversions once and harden scalar handling

.github/workflows/bindings.yml

@@ -41,3 +41,17 @@ jobs:
       - name: Test
         run: |
           yarn test:ci
+          ENABLE_PERSISTING=true yarn test:ci
+          yarn test:conversion
+          yarn test:conversion:mutations
+      - name: Generated conversion size budget
+        run: yarn size:conversion --check --output conversion-size.json
+      - name: Conversion benchmark (informational)
+        run: yarn bench:conversion:prepared --samples 7 --duration 20 --output conversion-benchmark.json
+      - uses: actions/upload-artifact@v4
+        with:
+          name: conversion-performance
+          path: |
+            packages/rescript-relay/conversion-benchmark.json
+            packages/rescript-relay/conversion-size.json
+            packages/rescript-relay/coverage/conversion/

.github/workflows/build-release.yml

@@ -20,7 +20,7 @@ jobs:
     name: Build PPX for release
     runs-on: ${{ matrix.platform }}
     container:
-      image: debian:bullseye
+      image: debian:bookworm
     defaults:
       run:
         working-directory: packages/rescript-relay/rescript-relay-ppx
@@ -34,6 +34,7 @@ jobs:
             curl \
             ca-certificates \
             git \
+            gnupg \
             unzip \
             bubblewrap
       - uses: actions/checkout@v4
@@ -47,7 +48,7 @@ jobs:
         with:
           ocaml-compiler: "4.14"
           dune-cache: true
-          cache-prefix: "v2-bullseye"
+          cache-prefix: "v3-bookworm"
       - name: Install PPX dependencies
         run: opam install . --deps-only
       - name: Build PPX

.github/workflows/ppx.yml

@@ -11,7 +11,7 @@ jobs:
     name: PPX Linux
     runs-on: ubuntu-latest
     container:
-      image: debian:bullseye
+      image: debian:bookworm
     steps:
       - name: Install system dependencies
         run: |
@@ -21,6 +21,7 @@ jobs:
             curl \
             ca-certificates \
             git \
+            gnupg \
             unzip \
             bubblewrap
       - uses: actions/checkout@v4
@@ -29,7 +30,7 @@ jobs:
         with:
           ocaml-compiler: "4.14"
           dune-cache: true
-          cache-prefix: "v2-bullseye"
+          cache-prefix: "v3-bookworm"
       - name: Install dependencies
         working-directory: packages/rescript-relay/rescript-relay-ppx
         run: opam install . --deps-only

.gitignore

@@ -12,4 +12,8 @@ _opam
 *.bs.js
 rescript-relay-compiler
 .now
-.DS_Store
+.DS_Store
+# Local conversion coverage and profiling output
+coverage/
+*.cpuprofile
+conversion-benchmark.json

CHANGELOG.md

@@ -1,5 +1,7 @@
 # master
 
+- Speed up response and variable conversion with prepared conversion plans and smaller generated instructions. Fix custom-scalar, nested-list, union, and plural `@catch` conversion edge cases; expand regression coverage and add performance/bundle-size benchmarks.
+
 - fix remove-unused-fields command. https://github.com/zth/rescript-relay/pull/636
 
 # 4.5.1

build-ci.sh

@@ -26,9 +26,8 @@ cp .npmignore ../../_release/;
 cp ../../README.md ../../_release/;
 cp ../../CHANGELOG.md ../../_release/;
 
-# copy and create mjs version of utils.js
-cp src/utils.js ../../_release/src/utils.mjs;
-sed -i 's/module\.exports \= /export /g' ../../_release/src/utils.mjs;
+# Build matching ESM conversion modules from the CommonJS source.
+node scripts/build-conversion-modules.js ../../_release/src;
 
 # copy config files
 echo "Copying config files..."

packages/rescript-relay/__tests__/Test_conversionContract-tests.js

@@ -0,0 +1,131 @@
+require("@testing-library/jest-dom/extend-expect");
+const React = require("react");
+const { render, screen, act } = require("@testing-library/react");
+const { RelayEnvironmentProvider } = require("react-relay");
+const { createMockEnvironment } = require("relay-test-utils");
+const { createOperationDescriptor } = require("relay-runtime");
+const Scalar = require("./Test_customScalars.bs");
+const Fragment = require("./Test_fragment.bs");
+const scalarNode =
+  require("./__generated__/TestCustomScalarsQuery_graphql.bs").node;
+const fragmentNode =
+  require("./__generated__/TestFragmentQuery_graphql.bs").node;
+
+function mount(environment, Component) {
+  const tree = () =>
+    React.createElement(
+      RelayEnvironmentProvider,
+      { environment },
+      React.createElement(
+        React.Suspense,
+        { fallback: "Loading" },
+        React.createElement(Component),
+      ),
+    );
+  const view = render(tree());
+  return () => view.rerender(tree());
+}
+
+test("compiled query converts nested scalar lists and unions after store updates without changing the store", () => {
+  const environment = createMockEnvironment();
+  const variables = {
+    number: [2],
+    beforeDate: new Date("2018-01-01T00:00:00Z"),
+  };
+  const wireVariables = Scalar.Query.convertVariables(variables);
+  expect(wireVariables).toEqual({
+    number: 2,
+    beforeDate: "2018-01-01T00:00:00.000Z",
+  });
+  const operation = createOperationDescriptor(scalarNode, wireVariables);
+  const date1 = "2020-01-01T00:00:00.000Z";
+  const date2 = "2021-01-01T00:00:00.000Z";
+  const payload = (date) => ({
+    loggedInUser: {
+      id: "viewer",
+      createdAt: date,
+      friends: [{ id: "friend", createdAt: date }],
+    },
+    member: { __typename: "User", id: "member", createdAt: date },
+  });
+  environment.commitPayload(operation, payload(date1));
+  let converted;
+  function Probe() {
+    converted = Scalar.Query.use(variables, "store-only");
+    return React.createElement(
+      "div",
+      null,
+      converted.loggedInUser.friends[0].createdAt.toISOString(),
+    );
+  }
+  const rerender = mount(environment, Probe);
+  expect(screen.getByText(date1)).toBeInTheDocument();
+  expect(converted.member.createdAt).toEqual(new Date(date1));
+  expect(
+    environment.lookup(operation.fragment).data.loggedInUser.friends[0]
+      .createdAt,
+  ).toBe(date1);
+  const first = converted;
+  rerender();
+  expect(converted).toBe(first); // The real binding's useMemo still applies.
+  act(() => environment.commitPayload(operation, payload(date2)));
+  expect(screen.getByText(date2)).toBeInTheDocument();
+  expect(converted).not.toBe(first);
+  expect(converted.member.createdAt).toEqual(new Date(date2));
+  expect(first.loggedInUser.friends[0].createdAt).toEqual(new Date(date1));
+  expect(environment.lookup(operation.fragment).data.member.createdAt).toBe(
+    date2,
+  );
+});
+
+test("compiled fragment refs can be consumed by nested and plural hooks across nullable store updates", () => {
+  const environment = createMockEnvironment();
+  const operation = createOperationDescriptor(fragmentNode, {});
+  const payload = (status) => ({
+    loggedInUser: {
+      id: "viewer",
+      firstName: "Viewer",
+      lastName: "Last",
+      onlineStatus: status,
+    },
+    users: {
+      edges: [
+        { node: { id: "friend", firstName: "Friend", onlineStatus: status } },
+      ],
+    },
+  });
+  environment.commitPayload(operation, payload("Online"));
+  let result;
+  function Probe() {
+    const query = Fragment.Query.use(undefined, "store-only");
+    const fragment = Fragment.Fragment.use(query.loggedInUser.fragmentRefs);
+    const nested = Fragment.SubFragment.use(fragment.fragmentRefs);
+    const plural = Fragment.PluralFragment.use(
+      query.users.edges.map((edge) => edge.node.fragmentRefs),
+    );
+    result = { query, fragment, nested, plural };
+    return React.createElement(
+      "div",
+      null,
+      `${nested.lastName}:${plural[0].onlineStatus ?? "none"}`,
+    );
+  }
+  mount(environment, Probe);
+  expect(screen.getByText("Last:Online")).toBeInTheDocument();
+  expect(result.fragment.updatableFragmentRefs).toBe(
+    result.fragment.fragmentRefs,
+  );
+  expect(result.query.users.edges[0].node.updatableFragmentRefs).toBe(
+    result.query.users.edges[0].node.fragmentRefs,
+  );
+  act(() => environment.commitPayload(operation, payload(null)));
+  expect(screen.getByText("Last:none")).toBeInTheDocument();
+  expect(result.fragment.onlineStatus).toBeUndefined();
+  expect(result.plural[0].onlineStatus).toBeUndefined();
+  // The refs still contain the wire value; only the returned data is converted.
+  expect(result.query.users.edges[0].node.fragmentRefs.onlineStatus).toBeNull();
+  expect(
+    environment.lookup(operation.fragment).data.users.edges[0].node
+      .onlineStatus,
+  ).toBeNull();
+});

packages/rescript-relay/__tests__/Test_conversionPlan-tests.js

@@ -0,0 +1,90 @@
+require("@testing-library/jest-dom/extend-expect");
+const React = require("react");
+const { render, screen, act } = require("@testing-library/react");
+const { RelayEnvironmentProvider } = require("react-relay");
+const { createMockEnvironment } = require("relay-test-utils");
+const { createOperationDescriptor } = require("relay-runtime");
+const { Query, ReverseQuery } = require("./Test_conversionPlan.bs");
+const generated = require("./__generated__/TestConversionPlanQuery_graphql.bs");
+
+const reverseGenerated = require("./__generated__/TestConversionPlanReverseQuery_graphql.bs");
+
+test.each([
+  ["original", Query, generated],
+  ["reversed", ReverseQuery, reverseGenerated],
+])("%s selection order preserves scalars, nulls and nested lists through mounted hooks and writes", (_, Query, generated) => {
+  const environment = createMockEnvironment();
+  const variables = {
+    input: {
+      grid: [[[1], undefined, [2]], undefined],
+      children: [{ grid: [[[3]]] }, undefined],
+    },
+  };
+  const convertedVariables = Query.convertVariables(variables);
+  expect(JSON.parse(JSON.stringify(convertedVariables))).toStrictEqual({
+    input: { grid: [[1, null, 2], null], children: [{ grid: [[3]] }, null] },
+  });
+  const operation = createOperationDescriptor(
+    generated.node,
+    convertedVariables,
+  );
+  const date = "2020-01-01T00:00:00.000Z";
+  const raw = {
+    conversionContract: {
+      grid: [[1, null, 2], null],
+      dates: [date, null],
+      raw: [[[null, { x: null }], null], null],
+      a_b: 7,
+      a: { b: date },
+    },
+  };
+  environment.commitPayload(operation, raw);
+  let data;
+  function Probe() {
+    data = Query.use(variables, "store-only");
+    return React.createElement(
+      "div",
+      null,
+      data.conversionContract.a?.b?.toISOString() ?? "empty",
+    );
+  }
+  render(
+    React.createElement(
+      RelayEnvironmentProvider,
+      { environment },
+      React.createElement(
+        React.Suspense,
+        { fallback: "loading" },
+        React.createElement(Probe),
+      ),
+    ),
+  );
+  expect(screen.getByText(date)).toBeInTheDocument();
+  expect(data.conversionContract).toStrictEqual({
+    grid: [[[1], undefined, [2]], undefined],
+    dates: [new Date(date), undefined],
+    raw: [[[null, { x: null }], undefined], undefined],
+    a_b: [7],
+    a: { b: new Date(date) },
+  });
+  expect(environment.lookup(operation.fragment).data).toStrictEqual(raw);
+  expect(generated.Internal.convertWrapResponse(data)).toStrictEqual(raw);
+  act(() =>
+    environment.commitPayload(operation, {
+      conversionContract: {
+        grid: null,
+        dates: null,
+        raw: null,
+        a_b: null,
+        a: null,
+      },
+    }),
+  );
+  expect(data.conversionContract).toStrictEqual({
+    grid: undefined,
+    dates: undefined,
+    raw: undefined,
+    a_b: undefined,
+    a: undefined,
+  });
+});

packages/rescript-relay/__tests__/Test_conversionPlan.res

@@ -0,0 +1,23 @@
+module Query = %relay(`
+  query TestConversionPlanQuery($input: ConversionContractInput) {
+    conversionContract(input: $input) {
+      grid
+      dates
+      raw
+      a_b
+      a { b }
+    }
+  }
+`)
+
+module ReverseQuery = %relay(`
+  query TestConversionPlanReverseQuery($input: ConversionContractInput) {
+    conversionContract(input: $input) {
+      a { b }
+      a_b
+      raw
+      dates
+      grid
+    }
+  }
+`)