fix(builder): deterministic ids for NumberCounter + form-relay marker

Two components of the det-id bug class were missed in the earlier migration:
NumberCounter's wrapper/counter ids and count-up script, and the shared
form-relay-wiring fid (used by ContactForm/FormContainer). Both used
Math.random() for exported HTML ids, breaking caching/diffing across exports.

Migrate both to scopeId(nodeId, fallbackSeed, prefix), threading nodeId
through NumberCounter.toHtml and relayFormWiring (via ContactForm.toHtml and
FormContainer.toHtml, both now passing nodeId as their 3rd arg).
This commit is contained in:
2026-07-12 14:43:31 -07:00
parent 177eda93d0
commit a036843728
8 changed files with 149 additions and 15 deletions
@@ -6,12 +6,12 @@ const toHtml = (ContactForm as any).toHtml;
describe('ContactForm.toHtml relay wiring', () => {
test('with recipientEmail: emits marker, placeholder action, honeypot', () => {
const { html } = toHtml({ recipientEmail: 'a@b.com', thankYouUrl: '/thx', fields: [] }, '');
expect(html).toMatch(/<!--WHP-FORM id="F[0-9a-z]+" recipient="a@b.com" thankyou="\/thx"-->/);
expect(html).toMatch(/action="__WHP_FORM_ACTION__F[0-9a-z]+__"/);
expect(html).toMatch(/<!--WHP-FORM id="F_[0-9a-z]+" recipient="a@b.com" thankyou="\/thx"-->/);
expect(html).toMatch(/action="__WHP_FORM_ACTION__F_[0-9a-z]+__"/);
expect(html).toContain('method="POST"');
expect(html).toContain('name="_gotcha"');
// marker id and action id match
const mid = html.match(/id="(F[0-9a-z]+)"/)![1];
const mid = html.match(/id="(F_[0-9a-z]+)"/)![1];
expect(html).toContain(`__WHP_FORM_ACTION__${mid}__`);
});
@@ -64,6 +64,28 @@ describe('ContactForm.toHtml successMessage', () => {
});
});
describe('ContactForm.toHtml relay marker deterministic + unique via node id (no Math.random)', () => {
test('same node id -> identical marker+placeholder ids across two calls', () => {
const { html: html1 } = toHtml({ recipientEmail: 'a@b.com', thankYouUrl: '/thx', fields: [] }, '', 'node-cf1');
const { html: html2 } = toHtml({ recipientEmail: 'a@b.com', thankYouUrl: '/thx', fields: [] }, '', 'node-cf1');
expect(html1).toBe(html2);
});
test('marker id always equals the placeholder id it pairs with', () => {
const { html } = toHtml({ recipientEmail: 'a@b.com', thankYouUrl: '/thx', fields: [] }, '', 'node-cf1');
const mid = html.match(/<!--WHP-FORM id="([^"]+)"/)![1];
expect(html).toContain(`action="__WHP_FORM_ACTION__${mid}__"`);
});
test('two different node ids -> different fids', () => {
const { html: html1 } = toHtml({ recipientEmail: 'a@b.com', thankYouUrl: '/thx', fields: [] }, '', 'node-cf1');
const { html: html2 } = toHtml({ recipientEmail: 'a@b.com', thankYouUrl: '/thx', fields: [] }, '', 'node-cf2');
const mid1 = html1.match(/<!--WHP-FORM id="([^"]+)"/)![1];
const mid2 = html2.match(/<!--WHP-FORM id="([^"]+)"/)![1];
expect(mid1).not.toBe(mid2);
});
});
describe('ContactForm.toHtml accessibility (F2.1)', () => {
const fields = [
{ type: 'text' as const, label: 'Name', name: 'name', placeholder: 'Your name', required: true },
+2 -2
View File
@@ -167,7 +167,7 @@ ContactForm.craft = {
/* ---------- HTML export ---------- */
(ContactForm as any).toHtml = (props: ContactFormProps, _childrenHtml: string) => {
(ContactForm as any).toHtml = (props: ContactFormProps, _childrenHtml: string, nodeId?: string) => {
const formStyle = cssPropsToString({
padding: '32px',
display: 'flex',
@@ -212,7 +212,7 @@ ContactForm.craft = {
alignSelf: 'flex-start',
});
const { marker, actionAttr, honeypot } = relayFormWiring(props.recipientEmail, props.thankYouUrl, props.formAction);
const { marker, actionAttr, honeypot } = relayFormWiring(props.recipientEmail, props.thankYouUrl, props.formAction, nodeId);
// The form-sender relay delivers success via a full-page 303 redirect
// (to thankYouUrl or a hosted thanks.php page) -- there is no in-page JS
@@ -6,14 +6,14 @@ const toHtml = (FormContainer as any).toHtml;
describe('FormContainer.toHtml relay wiring', () => {
test('with recipientEmail: marker + placeholder action + honeypot, forces POST', () => {
const { html } = toHtml({ recipientEmail: 'a@b.com', thankYouUrl: '/thx', method: 'GET' }, '<input name="email">');
expect(html).toMatch(/<!--WHP-FORM id="F[0-9a-z]+" recipient="a@b.com" thankyou="\/thx"-->/);
expect(html).toMatch(/action="__WHP_FORM_ACTION__F[0-9a-z]+__"/);
expect(html).toMatch(/<!--WHP-FORM id="F_[0-9a-z]+" recipient="a@b.com" thankyou="\/thx"-->/);
expect(html).toMatch(/action="__WHP_FORM_ACTION__F_[0-9a-z]+__"/);
expect(html).toContain('method="POST"'); // relay forces POST even though method=GET
expect(html).toContain('name="_gotcha"');
// honeypot precedes the form's children
expect(html.indexOf('_gotcha')).toBeLessThan(html.indexOf('name="email"'));
// marker id === action id
const mid = html.match(/id="(F[0-9a-z]+)"/)![1];
const mid = html.match(/id="(F_[0-9a-z]+)"/)![1];
expect(html).toContain(`__WHP_FORM_ACTION__${mid}__`);
});
@@ -24,4 +24,18 @@ describe('FormContainer.toHtml relay wiring', () => {
expect(html).toContain('action="/legacy"');
expect(html).toContain('<input name="email">');
});
test('same node id -> identical marker+placeholder ids across two calls', () => {
const { html: html1 } = toHtml({ recipientEmail: 'a@b.com', thankYouUrl: '/thx' }, '<input name="email">', 'node-fc1');
const { html: html2 } = toHtml({ recipientEmail: 'a@b.com', thankYouUrl: '/thx' }, '<input name="email">', 'node-fc1');
expect(html1).toBe(html2);
});
test('two different node ids -> different fids', () => {
const { html: html1 } = toHtml({ recipientEmail: 'a@b.com', thankYouUrl: '/thx' }, '<input name="email">', 'node-fc1');
const { html: html2 } = toHtml({ recipientEmail: 'a@b.com', thankYouUrl: '/thx' }, '<input name="email">', 'node-fc2');
const mid1 = html1.match(/<!--WHP-FORM id="([^"]+)"/)![1];
const mid2 = html2.match(/<!--WHP-FORM id="([^"]+)"/)![1];
expect(mid1).not.toBe(mid2);
});
});
+2 -2
View File
@@ -68,12 +68,12 @@ FormContainer.craft = {
/* ---------- HTML export ---------- */
(FormContainer as any).toHtml = (props: FormContainerProps, childrenHtml: string) => {
(FormContainer as any).toHtml = (props: FormContainerProps, childrenHtml: string, nodeId?: string) => {
const styleStr = cssPropsToString({
padding: '24px',
...props.style,
});
const { useRelay, marker, actionAttr, honeypot } = relayFormWiring(props.recipientEmail, props.thankYouUrl, props.action);
const { useRelay, marker, actionAttr, honeypot } = relayFormWiring(props.recipientEmail, props.thankYouUrl, props.action, nodeId);
const method = useRelay ? 'POST' : (props.method || 'POST'); // relay requires POST
const body = honeypot + childrenHtml; // honeypot as first child
return {
@@ -0,0 +1,49 @@
import { describe, test, expect } from 'vitest';
import { NumberCounter } from './NumberCounter';
const toHtml = (NumberCounter as any).toHtml;
const counters = [
{ number: 150, suffix: '+', label: 'Projects' },
{ number: 50, suffix: '+', label: 'Clients' },
];
describe('NumberCounter.toHtml deterministic + unique scope ids (thread node id, no Math.random)', () => {
test('same node id -> identical output across calls (deterministic)', () => {
const { html: html1 } = toHtml({ counters }, '', 'node-nc1');
const { html: html2 } = toHtml({ counters }, '', 'node-nc1');
expect(html1).toBe(html2);
});
test('different node ids -> distinct, non-colliding nc_ scopes (identical props, no collision)', () => {
const { html: html1 } = toHtml({ counters }, '', 'node-nc1');
const { html: html2 } = toHtml({ counters }, '', 'node-nc2');
const wrapId1 = html1.match(/<div id="(nc_[^"]+)"/)![1];
const wrapId2 = html2.match(/<div id="(nc_[^"]+)"/)![1];
expect(wrapId1).not.toBe(wrapId2);
});
test('wrapper id, per-counter ids, and inline script agree on the same uid', () => {
const { html } = toHtml({ counters }, '', 'node-nc1');
const wrapId = html.match(/<div id="(nc_[^"]+)"/)![1];
expect(html).toContain(`id="${wrapId}_n0"`);
expect(html).toContain(`id="${wrapId}_n1"`);
expect(html).toContain(`var uid="${wrapId}"`);
expect(html).toContain('document.getElementById(uid)');
expect(html).toContain('document.getElementById(uid+"_n"+i)');
});
test('no nodeId (legacy 2-arg call): still deterministic across repeated calls, not random', () => {
const { html: html1 } = toHtml({ counters }, '');
const { html: html2 } = toHtml({ counters }, '');
expect(html1).toBe(html2);
});
test('two different node ids never collide even with default (no counters override) props', () => {
const { html: html1 } = toHtml({}, '', 'node-a');
const { html: html2 } = toHtml({}, '', 'node-b');
const wrapId1 = html1.match(/<div id="(nc_[^"]+)"/)![1];
const wrapId2 = html2.match(/<div id="(nc_[^"]+)"/)![1];
expect(wrapId1).not.toBe(wrapId2);
});
});
@@ -1,7 +1,7 @@
import React, { CSSProperties } from 'react';
import { useNode, UserComponent } from '@craftjs/core';
import { cssPropsToString } from '../../utils/style-helpers';
import { escapeHtml, escapeAttr } from '../../utils/escape';
import { escapeHtml, escapeAttr, scopeId } from '../../utils/escape';
interface Counter {
number: number;
@@ -109,7 +109,7 @@ NumberCounter.craft = {
/* ---------- HTML export ---------- */
(NumberCounter as any).toHtml = (props: NumberCounterProps, _childrenHtml: string) => {
(NumberCounter as any).toHtml = (props: NumberCounterProps, _childrenHtml: string, nodeId?: string) => {
const {
counters = defaultCounters,
columns = 4,
@@ -120,7 +120,13 @@ NumberCounter.craft = {
} = props;
const items = counters.length > 0 ? counters : defaultCounters;
const uid = 'nc_' + Math.random().toString(36).slice(2, 8);
// Deterministic AND unique id for this counter instance's wrapper/span
// ids and getElementById() calls inside its inline script -- scoped on
// the Craft node id so two NumberCounter instances (e.g. both left at
// default props) don't collide and end up animating each other's digits.
const seed = items.map((c) => `${c.number}${c.suffix}::${c.label}`).join('|');
const uid = scopeId(nodeId, seed, 'nc');
const sectionStyle = cssPropsToString({
padding: '60px 20px',
+38
View File
@@ -0,0 +1,38 @@
import { describe, test, expect } from 'vitest';
import { relayFormWiring } from './form-relay-wiring';
describe('relayFormWiring deterministic + unique fid (thread node id, no Math.random)', () => {
test('no recipient: no relay, no marker, no fid needed', () => {
const wiring = relayFormWiring(undefined, undefined, '/legacy', 'node-1');
expect(wiring.useRelay).toBe(false);
expect(wiring.marker).toBe('');
});
test('same node id -> identical marker + placeholder ids across two calls (deterministic)', () => {
const w1 = relayFormWiring('a@b.com', '/thx', undefined, 'node-form1');
const w2 = relayFormWiring('a@b.com', '/thx', undefined, 'node-form1');
expect(w1.marker).toBe(w2.marker);
expect(w1.actionAttr).toBe(w2.actionAttr);
});
test('marker id and placeholder id always match each other', () => {
const w = relayFormWiring('a@b.com', '/thx', undefined, 'node-form1');
const mid = w.marker.match(/id="([^"]+)"/)![1];
expect(w.actionAttr).toBe(`__WHP_FORM_ACTION__${mid}__`);
});
test('different node ids -> different fids (no collision)', () => {
const w1 = relayFormWiring('a@b.com', '/thx', undefined, 'node-form1');
const w2 = relayFormWiring('a@b.com', '/thx', undefined, 'node-form2');
const mid1 = w1.marker.match(/id="([^"]+)"/)![1];
const mid2 = w2.marker.match(/id="([^"]+)"/)![1];
expect(mid1).not.toBe(mid2);
});
test('no nodeId (legacy call): still deterministic across repeated calls, not random', () => {
const w1 = relayFormWiring('a@b.com', '/thx', undefined);
const w2 = relayFormWiring('a@b.com', '/thx', undefined);
expect(w1.marker).toBe(w2.marker);
expect(w1.actionAttr).toBe(w2.actionAttr);
});
});
+7 -2
View File
@@ -9,7 +9,7 @@
* HTML — see PR #47 review).
*/
import { escapeAttr, safeUrl } from './escape';
import { escapeAttr, safeUrl, scopeId } from './escape';
export interface RelayWiring {
/** true when a recipient is set (relay path); false = legacy formAction fallback */
@@ -26,16 +26,21 @@ export interface RelayWiring {
* @param recipientEmail the "Send submissions to" address (empty/undefined = no relay)
* @param thankYouUrl optional post-submit redirect (blank = hosted thank-you page)
* @param fallbackAction the form's existing action to use when no recipient is set
* @param nodeId the Craft node id of the calling form component, used to scope
* the marker/placeholder id deterministically and uniquely --
* see `scopeId` in ./escape. Falls back to a stable hash of the
* recipient/thankYouUrl/fallbackAction when omitted (never random).
*/
export function relayFormWiring(
recipientEmail: string | undefined,
thankYouUrl: string | undefined,
fallbackAction: string | undefined,
nodeId?: string,
): RelayWiring {
if (!recipientEmail) {
return { useRelay: false, marker: '', actionAttr: escapeAttr(safeUrl(fallbackAction || '#')), honeypot: '' };
}
const fid = 'F' + Math.random().toString(36).slice(2, 8);
const fid = scopeId(nodeId, `${recipientEmail}::${thankYouUrl || ''}::${fallbackAction || ''}`, 'F');
return {
useRelay: true,
marker: `<!--WHP-FORM id="${fid}" recipient="${escapeAttr(recipientEmail)}" thankyou="${escapeAttr(thankYouUrl || '')}"-->`,