refactor: turn on strict, fix React 19 declarations, type-test both majors

.github/workflows/ci.yml

@@ -63,6 +63,16 @@ jobs:
       - name: Build
         run: pnpm run build
 
+      # Compiles a consumer against the built lib/index.d.ts under every
+      # supported @types/react major, with skipLibCheck off. Has to run after
+      # the build, since the declarations are what it type checks. type-check
+      # above cannot catch this: it checks src/ against the one React the root
+      # has installed, and the demo resolves the library through a source alias,
+      # so neither ever reads the published declarations. React 19 removing the
+      # global JSX namespace broke them with both of those still green.
+      - name: Type-test published declarations
+        run: pnpm run test:types
+
       # The demo consumes the library straight from src, so it catches breakage
       # that the library's own build and tests do not.
       - name: Build demo

CHANGELOG.md

@@ -1,5 +1,24 @@
 ## Unreleased
 
+### bug fixes
+
+- **the published type declarations now compile under React 19.** React 19
+  removed the global `JSX` namespace and stopped exporting `ReactSVG`, both of
+  which `index.d.ts` referenced, so anyone on `@types/react` 19 without
+  `skipLibCheck` could not build against this package. Every runtime code path
+  already worked on 19. CI now compiles a consumer against the built
+  declarations under both supported `@types/react` majors.
+- a custom `headShape` or `tailShape` with no `svgElem` rendered the literal
+  text "path" instead of an arrowhead, and parsing wrote its defaults into the
+  object it was given - the caller's own for a custom shape, the shared
+  built-in for a shape name.
+- `dashness={{ strokeLen: n }}` rendered `stroke-dasharray="n undefined"`,
+  because `nonStrokeLen` is optional and was passed straight through. It now
+  falls back to `strokeWidth`. Nothing else about how the two lengths are read
+  changed: `{ nonStrokeLen: n }` on its own is still ignored, and a length of
+  `0` is still treated as absent, so no arrow that renders correctly today
+  renders differently.
+
 ### features
 
 - **`gridRadius`** rounds the corners of a `path='grid'` arrow. `gridRadius`

__test__/propParsing.test.tsx

@@ -0,0 +1,140 @@
+import { render } from '@testing-library/react';
+import { afterEach, describe, expect, it, vi } from 'vitest';
+import Xarrow from '../src/Xarrow/Xarrow';
+import Xwrapper from '../src/Xwrapper';
+
+/**
+ * Guards the prop-parsing behaviour that the type work changed on purpose.
+ * Everything else about the rendered output was verified byte for byte against
+ * the previous release across a prop matrix, so these cover only the cases that
+ * moved.
+ */
+
+const geom: Record<string, Partial<DOMRect>> = {
+  'box-a': { left: 40, top: 30, right: 140, bottom: 90, width: 100, height: 60, x: 40, y: 30 },
+  'box-b': { left: 400, top: 260, right: 500, bottom: 320, width: 100, height: 60, x: 400, y: 260 },
+};
+
+const placeBoxes = () =>
+  vi.spyOn(Element.prototype, 'getBoundingClientRect').mockImplementation(function (this: Element) {
+    const r = geom[this.id] ?? { left: 0, top: 0, right: 0, bottom: 0, width: 0, height: 0, x: 0, y: 0 };
+    return { ...r, toJSON: () => r } as DOMRect;
+  });
+
+const Boxes = ({ ...arrowProps }) => (
+  <Xwrapper>
+    <div id="box-a">A</div>
+    <div id="box-b">B</div>
+    <Xarrow start="box-a" end="box-b" {...arrowProps} />
+  </Xwrapper>
+);
+
+describe('prop parsing', () => {
+  afterEach(() => vi.restoreAllMocks());
+
+  describe('edge shapes', () => {
+    it('falls back to the default shape when a custom shape has no svgElem', () => {
+      placeBoxes();
+      // Used to default to the bare string 'path', which React renders as the
+      // literal text "path" instead of an arrowhead.
+      const { container } = render(<Boxes headShape={{ offsetForward: 0.3 }} />);
+
+      expect(container.textContent).not.toContain('path');
+      expect(container.querySelectorAll('svg g path').length).toBeGreaterThan(0);
+    });
+
+    it('does not write defaults into the shape object it was given', () => {
+      placeBoxes();
+      const custom = { offsetForward: 0.3 };
+      render(<Boxes headShape={custom} />);
+
+      // Parsing used to fill svgElem in on this object. For a built-in shape
+      // name the same code path wrote into the shared arrowShapes constant.
+      expect(custom).toEqual({ offsetForward: 0.3 });
+    });
+
+    it('leaves an explicitly null field alone, as before', () => {
+      placeBoxes();
+      // Only an absent svgElem falls back. A null one is passed through the way
+      // the previous release did, rather than being reinterpreted.
+      const { container } = render(<Boxes headShape={{ svgElem: null } as never} />);
+
+      expect(container.querySelectorAll('svg g path').length).toBe(0);
+    });
+
+    it('keeps a custom offsetForward of 0 rather than defaulting it', () => {
+      placeBoxes();
+      const { container } = render(<Boxes headShape={{ svgElem: <circle r={0.5} />, offsetForward: 0 }} />);
+
+      expect(container.querySelector('svg g circle')).not.toBeNull();
+    });
+  });
+
+  describe('dashness', () => {
+    const dashArray = (props: Record<string, unknown>) => {
+      placeBoxes();
+      const { container } = render(<Boxes strokeWidth={4} {...props} />);
+      return container.querySelector('svg path')?.getAttribute('stroke-dasharray');
+    };
+
+    // Only one dashness behaviour changed: nonStrokeLen used to be passed
+    // through undefined when strokeLen was given. Everything else about how the
+    // two lengths are read is deliberately left as it was, so that no input
+    // that already rendered something valid renders something else now.
+    it('fills in nonStrokeLen when only strokeLen is given', () => {
+      // Used to render "10 undefined".
+      expect(dashArray({ dashness: { strokeLen: 10 } })).toBe('10 4');
+    });
+
+    it('keeps both lengths when both are given', () => {
+      expect(dashArray({ dashness: { strokeLen: 10, nonStrokeLen: 5 } })).toBe('10 5');
+      expect(dashArray({ dashness: { strokeLen: 10, nonStrokeLen: 0 } })).toBe('10 0');
+    });
+
+    it('still ignores nonStrokeLen given on its own, as before', () => {
+      // Arguably it should be honoured, but changing that would repaint arrows
+      // that render fine today, so it is left alone.
+      expect(dashArray({ dashness: { nonStrokeLen: 10 } })).toBe('8 4');
+    });
+
+    it('still treats a zero length as absent, as before', () => {
+      expect(dashArray({ dashness: { strokeLen: 0 } })).toBe('8 4');
+      expect(dashArray({ dashness: { nonStrokeLen: 0 } })).toBe('8 4');
+    });
+
+    it('defaults both lengths from strokeWidth when neither is given', () => {
+      expect(dashArray({ dashness: {} })).toBe('8 4');
+      expect(dashArray({ dashness: true })).toBe('8 4');
+    });
+
+    it('treats animation: true as the documented one second default', () => {
+      placeBoxes();
+      // Used to store the boolean and rely on `1 / true` being 1.
+      const { container } = render(<Boxes dashness={{ strokeLen: 8, animation: true }} />);
+
+      const anim = container.querySelector('svg path animate');
+      expect(anim?.getAttribute('dur')).toBe('1s');
+    });
+  });
+
+  describe('anchors', () => {
+    const anchors = ['auto', 'left', 'right', 'top', 'bottom', 'middle'] as const;
+    const paths = ['smooth', 'grid', 'straight'] as const;
+
+    // The curve is picked by a key built from both anchor positions. That key
+    // used to be assembled by string concatenation, so a position matching none
+    // of the branches produced a key with no entry and threw.
+    anchors.forEach((startAnchor) =>
+      paths.forEach((path) => {
+        it(`renders a valid path for startAnchor=${startAnchor} path=${path}`, () => {
+          placeBoxes();
+          const { container } = render(<Boxes startAnchor={startAnchor} path={path} />);
+
+          const d = container.querySelector('svg path')?.getAttribute('d') ?? '';
+          expect(d).toMatch(/^M /);
+          expect(d).not.toMatch(/NaN|undefined/);
+        });
+      }),
+    );
+  });
+});

package.json

@@ -27,6 +27,7 @@
     "build": "pnpm run clean && vite build",
     "build:watch": "vite build --watch",
     "type-check": "tsc --noEmit",
+    "test:types": "pnpm --filter react-xarrows-type-tests run test",
     "lint": "eslint .",
     "lint:fix": "eslint . --fix",
     "format": "prettier --write .",

pnpm-workspace.yaml

@@ -1,6 +1,7 @@
 packages:
   - '.'
   - 'examples'
+  - 'type-tests'
 
 onlyBuiltDependencies:
   - esbuild

src/Xarrow/Xarrow.tsx

@@ -5,7 +5,7 @@
 import { getPosition } from './utils/GetPosition';
 import { getTotalLength } from './utils';
 
 const log = console.log;
 
 const Xarrow: React.FC<xarrowPropsType> = (props: xarrowPropsType) => {
   // log('xarrow update');
@@ -19,7 +19,7 @@
     lineDashAnimRef: useRef<SVGElement>(null),
     headOpacityAnimRef: useRef<SVGElement>(null),
   });
   const { svgRef, lineRef, headRef, tailRef, lineDrawAnimRef, lineDashAnimRef, headOpacityAnimRef } = mainRef.current;
   useContext(XarrowContext);
   const xProps = useXarrowProps(props, mainRef.current);
   const [propsRefs] = xProps;
@@ -56,7 +56,9 @@
   const [, setRender] = useState({});
   const forceRerender = () => setRender({});
 
-  const [st, setSt] = useState({
+  // Typed from what getPosition returns rather than inferred from the literal
+  // below, which would make the two empty arrays never[].
+  const [st, setSt] = useState<ReturnType<typeof getPosition>>({
     //initial state
     cx0: 0, //x start position of the canvas
     cy0: 0, //y start position of the canvas
@@ -160,9 +162,9 @@
 
       const handleDrawAmimEnd = () => {
         setDrawAnimEnded(true);
         // @ts-ignore
         headOpacityAnimRef.current?.beginElement();
         // @ts-ignore
         lineDashAnimRef.current?.beginElement();
       };
       // Deliberately does not depend on the arrowhead. This listener is what
@@ -206,7 +208,7 @@
               left: st.cx0,
               top: st.cy0,
               pointerEvents: 'none',
-              border: _debug ? '1px dashed yellow' : null,
+              border: _debug ? '1px dashed yellow' : undefined,
               ...SVGcanvasStyle,
             }}
             overflow="auto"
@@ -221,7 +223,7 @@
               strokeWidth={strokeWidth}
               fill="transparent"
               pointerEvents="visibleStroke"
               {...(passProps as any)}
               {...arrowBodyProps}>
               <>
                 {drawAnimEnded || !animateDrawing ? (
@@ -260,7 +262,7 @@
                 fill={tailColor}
                 pointerEvents="auto"
                 transform={`translate(${xOffsetTail},${yOffsetTail}) rotate(${st.tailOrient}) scale(${st.fTailSize})`}
                 {...(passProps as any)}
                 {...arrowTailProps}>
                 {tailShape.svgElem}
               </g>
@@ -269,13 +271,13 @@
             {/* head of the arrow */}
             {showHead ? (
               <g
                 ref={headRef as any}
                 // d={normalArrowShape}
                 fill={headColor}
                 pointerEvents="auto"
                 transform={`translate(${xOffsetHead},${yOffsetHead}) rotate(${st.headOrient}) scale(${st.fHeadSize})`}
                 opacity={animateDrawing && !drawAnimEnded ? 0 : 1}
                 {...(passProps as any)}
                 {...arrowHeadProps}>
                 {/* No repeatCount: SMIL rejects 0 and logs "Unexpected value 0
                     parsing repeatCount attribute", and the default of a single

src/Xarrow/anchors.ts

@@ -1,5 +1,4 @@
-import { anchorCustomPositionType } from '../types';
-import { dimensionType } from '../privateTypes';
+import { dimensionType, parsedAnchorType } from '../privateTypes';
 
 const getAnchorsDefaultOffsets = (width: number, height: number) => {
   return {
@@ -11,7 +10,7 @@ const getAnchorsDefaultOffsets = (width: number, height: number) => {
   };
 };
 
-export const calcAnchors = (anchors: anchorCustomPositionType[], anchorPos: dimensionType) => {
+export const calcAnchors = (anchors: parsedAnchorType[], anchorPos: dimensionType) => {
   // now prepare this list of anchors to object expected by the `getShortestLine` function
   return anchors.map((anchor) => {
     const defsOffsets = getAnchorsDefaultOffsets(anchorPos.right - anchorPos.x, anchorPos.bottom - anchorPos.y);