Name placeholders with a single-key object, and keep compiled values out of the descriptor's namespace

.changeset/brave-moons-repeat.md

@@ -0,0 +1,7 @@
+---
+'@saykit/transform-js': minor
+'@saykit/react': minor
+'saykit': minor
+---
+
+Compile message values behind an underscore so a value named `id` no longer displaces the message id

.changeset/olive-hounds-invent.md

@@ -0,0 +1,7 @@
+---
+'@saykit/transform-jsx': minor
+'@saykit/transform-js': minor
+'saykit': minor
+---
+
+Name a placeholder by interpolating a single-key object, `${{ cartTotal: getTotal() }}`

.changeset/olive-pans-repeat.md

@@ -0,0 +1,7 @@
+---
+'@saykit/transform-jsx': minor
+'@saykit/transform-js': minor
+'@saykit/config': minor
+---
+
+Reject two different values sharing a placeholder name, and allow a repeat when they are identical

packages/config/src/features/messages/identifier.test.ts

@@ -111,15 +111,44 @@ describe('assignSequenceIdentifiers', () => {
     );
   });
 
-  it('allows two arguments to share an identifier', () => {
+  it('allows two arguments to share a name when they are equivalent', () => {
     const message = new CompositeMessage(
       {},
       [],
       [],
-      [new ArgumentMessage('total', null), new ArgumentMessage('total', null)],
+      [new ArgumentMessage('total', 'sum'), new ArgumentMessage('total', 'sum')],
       null,
     );
-    expect(() => assignSequenceIdentifiers(message)).not.toThrow();
+    expect(() =>
+      assignSequenceIdentifiers(message, { current: 0 }, (a, b) => a === b),
+    ).not.toThrow();
+  });
+
+  it('throws when arguments sharing a name are not equivalent', () => {
+    const message = new CompositeMessage(
+      {},
+      [],
+      [],
+      [new ArgumentMessage('total', 'sum'), new ArgumentMessage('total', 'other')],
+      null,
+    );
+    expect(() => assignSequenceIdentifiers(message, { current: 0 }, (a, b) => a === b)).toThrow(
+      "Duplicate placeholder name 'total', give each value in a message its own name unless they are identical",
+    );
+  });
+
+  it('compares a choice against an argument of the same name', () => {
+    const choice = new ChoiceMessage('plural', 'count', [], 'n');
+    const message = new CompositeMessage(
+      {},
+      [],
+      [],
+      [choice, new ArgumentMessage('count', 'other')],
+      null,
+    );
+    expect(() => assignSequenceIdentifiers(message, { current: 0 }, (a, b) => a === b)).toThrow(
+      "Duplicate placeholder name 'count'",
+    );
   });
 
   it('throws when an element tag collides with an argument', () => {

packages/config/src/features/messages/identifier.ts

@@ -9,16 +9,16 @@ import {
 export const AUTO_INCREMENT_IDENTIFIER = Symbol('auto-increment');
 
 /**
- * Decides whether two elements sharing a tag are the same element, and so may
- * share it. Only the syntax that produced them can answer that, so the caller
- * supplies the comparison; by default no two elements are interchangeable.
+ * Decides whether two placeholders sharing a name are the same placeholder, and
+ * so may share it. Only the syntax that produced them can answer that, so the
+ * caller supplies the comparison; by default nothing is interchangeable.
  */
-export type ElementEquivalence = (a: any, b: any) => boolean;
+export type PlaceholderEquivalence = (a: any, b: any) => boolean;
 
 export function assignSequenceIdentifiers(
   message: Message,
   sequence = { current: 0 },
-  equivalent: ElementEquivalence = () => false,
+  equivalent: PlaceholderEquivalence = () => false,
 ) {
   const reserved = collectAssignedIdentifiers(message, equivalent);
 
@@ -51,30 +51,45 @@ export function assignSequenceIdentifiers(
  * Collect the identifiers already assigned before this pass runs, so generated
  * sequence numbers never shadow an explicit one (e.g. an element tagged `0`).
  *
- * Elements that differ need their own tag: they each compile to their own prop,
- * and a translator has to be able to tell them apart. Repeats are fine when
- * nothing distinguishes them — two identical elements are one prop, the same
- * way the same variable interpolated twice is one value — so arguments are
- * never checked and elements only when they are not equivalent.
+ * A name is claimed by what produced it, not by the name alone. Two that differ
+ * each compile to their own prop, and a translator moving them around a sentence
+ * has to be able to tell them apart, so they are a build error. Repeats are fine
+ * when nothing distinguishes them: the same variable interpolated twice is one
+ * value, and two identical elements are one tag.
  */
-function collectAssignedIdentifiers(message: Message, equivalent: ElementEquivalence) {
-  const tags = new Map<string, ElementMessage>();
-  const values = new Set<string>();
+function collectAssignedIdentifiers(message: Message, equivalent: PlaceholderEquivalence) {
+  const tags = new Map<string, unknown>();
+  const values = new Map<string, unknown>();
+
+  function claim(
+    claimed: Map<string, unknown>,
+    identifier: string,
+    expression: unknown,
+    conflict: string,
+  ) {
+    if (claimed.has(identifier) && !equivalent(claimed.get(identifier), expression))
+      throw new Error(conflict);
+    claimed.set(identifier, expression);
+  }
 
   function walk(message: Message) {
-    if (message instanceof ElementMessage && typeof message.identifier === 'string') {
-      const previous = tags.get(message.identifier);
-      if (previous && !equivalent(previous.expression, message.expression))
-        throw new Error(
-          `Duplicate element tag '${message.identifier}', give each element in a message its own tag unless they are identical`,
-        );
-      tags.set(message.identifier, message);
-    }
+    if (message instanceof ElementMessage && typeof message.identifier === 'string')
+      claim(
+        tags,
+        message.identifier,
+        message.expression,
+        `Duplicate element tag '${message.identifier}', give each element in a message its own tag unless they are identical`,
+      );
     if (
       (message instanceof ArgumentMessage || message instanceof ChoiceMessage) &&
       typeof message.identifier === 'string'
     )
-      values.add(message.identifier);
+      claim(
+        values,
+        message.identifier,
+        message.expression,
+        `Duplicate placeholder name '${message.identifier}', give each value in a message its own name unless they are identical`,
+      );
     if (message instanceof CompositeMessage || message instanceof ElementMessage)
       for (const child of message.children) walk(child);
     if (message instanceof ChoiceMessage) for (const branch of message.branches) walk(branch.value);
@@ -86,5 +101,5 @@ function collectAssignedIdentifiers(message: Message, equivalent: ElementEquival
     if (values.has(tag))
       throw new Error(`Element tag '${tag}' collides with an argument of the same name`);
 
-  return new Set([...tags.keys(), ...values]);
+  return new Set([...tags.keys(), ...values.keys()]);
 }

packages/integration-react/src/runtime/index.test.ts

@@ -40,10 +40,11 @@ describe('Say', () => {
     };
     expect(props.html).toBe('<name/> world');
     expect(props.whitespace).toBe(false);
-    // The descriptor passed to `say.call` is exactly the message id and the
-    // value props unprefixed, so a prefixed key or a renderer prop leaking
-    // into it fails here rather than reaching the runtime.
-    expect(call).toHaveBeenCalledWith({ id: 'greet', name: bold });
+    // The value props reach `say.call` still prefixed — the runtime does the
+    // single strip — with the id merged in and `whitespace` removed. Asserted
+    // exactly, so a renderer prop leaking into the descriptor fails here
+    // rather than reaching the runtime.
+    expect(call).toHaveBeenCalledWith({ id: 'greet', _name: bold });
 
     // A slot that maps to a valid element clones it; other slots pass the tag through.
     const resolved = props.components('name') as (p: object) => ReactElement;
@@ -72,15 +73,30 @@ describe('Say', () => {
       whitespace?: boolean;
       components: (tag?: string) => unknown;
     };
-    // The real id still reaches `say.call` and the tag named after it does not
-    // displace it, while a tag named `whitespace` is a value like any other and
-    // the flag itself still reaches the renderer.
-    expect(call).toHaveBeenCalledWith({ id: 'greet', whitespace: bold });
+    // The real id still reaches `say.call` and the value named after it does
+    // not displace it, since the two live in different namespaces until the
+    // runtime strips one underscore. The `whitespace` flag is destructured out
+    // for the renderer, while a value of that name rides along untouched.
+    expect(call).toHaveBeenCalledWith({ id: 'greet', _id: bold, _whitespace: bold });
     expect(props.whitespace).toBe(false);
     expect(typeof props.components('id')).toBe('function');
     expect(typeof props.components('whitespace')).toBe('function');
   });
 
+  it('leaves a tag whose name starts with an underscore intact', () => {
+    const call = vi.fn(() => '<_link/>');
+    globalThis.GET_SAY = () => ({ call });
+
+    const link = createElement('a', null, 'here');
+    const element = (Say as (p: unknown) => ReactElement)({ id: 'greet', __link: link });
+
+    const props = element.props as { components: (tag?: string) => unknown };
+    // One strip in the runtime and one in the resolver, each on its own copy:
+    // `__link` reaches `say.call` untouched and resolves as `_link` for the tag.
+    expect(call).toHaveBeenCalledWith({ id: 'greet', __link: link });
+    expect(typeof props.components('_link')).toBe('function');
+  });
+
   it('exposes Plural, Ordinal and Select macros that throw', () => {
     expect(() => Say.Plural({ _: 1, other: '#' })).toThrow("'Say.Plural' is a macro");
     expect(() => Say.Ordinal({ _: 1, other: '#' })).toThrow("'Say.Ordinal' is a macro");

packages/integration-react/src/runtime/index.ts

@@ -6,7 +6,7 @@ import {
   type ReactElement,
   type ReactNode,
 } from 'react';
-import type { Disallow, NumeralOptions, SelectOptions } from 'saykit';
+import type { Disallow, Named, NumeralOptions, SelectOptions } from 'saykit';
 import { Renderer } from '~/components/renderer.js';
 import { type PropsWithJSXSafeKeys, resolveValuePropKeys } from '~/types.js';
 
@@ -34,9 +34,11 @@ export function Say(props: { id: string; whitespace?: boolean; [match: string]:
   const values = resolveValuePropKeys(rest);
 
   return createElement(Renderer, {
-    // The id is kept out of `values` and merged in last, so a message free to
-    // name a tag `id` still cannot displace the message being looked up.
-    html: say.call({ ...values, id }),
+    // The props go through still prefixed: `Say#call` does the single strip for
+    // every caller, so there is nowhere for it to happen twice. The id is
+    // merged in last, so a message free to name a value `id` still cannot
+    // displace the message being looked up.
+    html: say.call({ ...rest, id }),
     whitespace,
     components(tag?: string) {
       if (tag && tag in values && isValidElement(values[tag])) {
@@ -70,7 +72,9 @@ export namespace Say {
    * @remark This is a macro and must be used with the relevant saykit plugin
    */
   export function Plural(
-    props: { _: number } & PropsWithJSXSafeKeys<Disallow<NumeralOptions, 'id' | 'context'>>,
+    props: { _: number | Named<number> } & PropsWithJSXSafeKeys<
+      Disallow<NumeralOptions, 'id' | 'context'>
+    >,
   ): ReactNode {
     void props;
     throw new Error("'Say.Plural' is a macro and must be used with the relevant saykit plugin");
@@ -96,7 +100,9 @@ export namespace Say {
    * @remark This is a macro and must be used with the relevant saykit plugin
    */
   export function Ordinal(
-    props: { _: number } & PropsWithJSXSafeKeys<Disallow<NumeralOptions, 'id' | 'context'>>,
+    props: { _: number | Named<number> } & PropsWithJSXSafeKeys<
+      Disallow<NumeralOptions, 'id' | 'context'>
+    >,
   ): ReactNode {
     void props;
     throw new Error("'Say.Ordinal' is a macro and must be used with the relevant saykit plugin");
@@ -121,7 +127,9 @@ export namespace Say {
    * @remark This is a macro and must be used with the relevant saykit plugin
    */
   export function Select(
-    props: { _: string } & PropsWithJSXSafeKeys<Disallow<SelectOptions, 'id' | 'context'>>,
+    props: { _: string | Named<string> } & PropsWithJSXSafeKeys<
+      Disallow<SelectOptions, 'id' | 'context'>
+    >,
   ): ReactNode {
     void props;
     throw new Error("'Say.Select' is a macro and must be used with the relevant saykit plugin");

packages/integration/src/runtime.test.ts

@@ -5,7 +5,13 @@ import { Say } from './runtime.js';
 type Locale = 'en' | 'fr' | 'de';
 
 const messages = {
-  en: { greeting: 'Hello', items: '{count, plural, one {# item} other {# items}}' },
+  en: {
+    greeting: 'Hello',
+    items: '{count, plural, one {# item} other {# items}}',
+    named: 'Hello, {name}',
+    identified: 'Order {id}',
+    underscored: 'Total {_total}',
+  },
   fr: { greeting: 'Bonjour', items: '{count, plural, one {# article} other {# articles}}' },
 } satisfies Partial<Record<Locale, Say.Messages>>;
 
@@ -20,6 +26,14 @@ const opts = (o: {
   loader?: Say.Loader<Locale>;
 }) => o as Say.Options<Locale, Say.Loader<Locale> | undefined>;
 
+/**
+ * Drop the bidi isolation marks the formatter wraps substituted values in, so
+ * an assertion can read as the sentence a user sees.
+ */
+function plain(formatted: string) {
+  return formatted.replaceAll(/[⁨⁩]/g, '');
+}
+
 function make(active?: Locale) {
   const say = new Say<Locale>(opts({ locales: ['en', 'fr', 'de'], messages }));
   if (active) say.activate(active);
@@ -249,6 +263,46 @@ describe('Say.call', () => {
   it('throws when the message id is not found', () => {
     expect(() => make('en').call({ id: 'missing' })).toThrow('Message for missing is not a string');
   });
+
+  it('strips the underscore the transform compiles values behind', () => {
+    expect(plain(make('en').call({ id: 'named', _name: 'Ada' }))).toBe('Hello, Ada');
+  });
+
+  it('formats keys written without one, so a hand-written call still works', () => {
+    expect(plain(make('en').call({ id: 'named', name: 'Ada' }))).toBe('Hello, Ada');
+  });
+
+  it('formats a value named after the descriptor id', () => {
+    // The lookup still uses the id; the value only fills `{id}` in the message.
+    expect(plain(make('en').call({ id: 'identified', _id: '42' }))).toBe('Order 42');
+  });
+
+  it('does not expose the message id as a value', () => {
+    // `{id}` is left unresolved rather than filled with the message's own id.
+    expect(plain(make('en').call({ id: 'identified' }))).not.toContain('identified');
+  });
+
+  it('strips exactly one underscore, so a name that starts with one survives', () => {
+    expect(plain(make('en').call({ id: 'underscored', __total: '9' }))).toBe('Total 9');
+  });
+
+  it('treats a value named `__proto__` as a value, not as a prototype', () => {
+    // Assigning the stripped key would write through to `Object.prototype`
+    // rather than naming a placeholder, so the values are built from own
+    // entries instead.
+    const descriptor = { id: 'named', _name: 'Ada' };
+    Object.defineProperty(descriptor, '___proto__', {
+      value: { polluted: true },
+      enumerable: true,
+    });
+    expect(plain(make('en').call(descriptor))).toBe('Hello, Ada');
+    expect(({} as { polluted?: boolean }).polluted).toBeUndefined();
+  });
+
+  it('ignores keys a descriptor only inherits', () => {
+    const descriptor = Object.assign(Object.create({ _name: 'Ghost' }), { id: 'named' });
+    expect(plain(make('en').call(descriptor))).not.toContain('Ghost');
+  });
 });
 
 describe('Say inspect', () => {