From aeaa97e5e35ee75b6e73de4308dc9708161c3fed Mon Sep 17 00:00:00 2001 From: Peyton-Spencer Date: Mon, 10 Aug 2026 03:20:47 -0400 Subject: [PATCH] fix: extract only bindings imported from solid-translate (or i18n re-exports) The AST extractor treated any msg("...") call and any / element as an extraction marker by name alone. A callback parameter named msg (e.g. Solid's {(msg) => ...}) produced spurious "msg() called without arguments" warnings, and a local function named msg could get its string arguments wrongly extracted into the catalog. Extraction now resolves the identifier's binding: imports must come from "solid-translate" or an accepted re-export wrapper (default: any specifier whose final path segment is solid-translate or i18n; configurable via extractImportSources), aliased imports resolve to their imported name, and locally bound identifiers (params, local consts, local components, with scope shadowing tracked) are never extracted. Unbound identifiers keep the old name-based behavior for snippet-style sources. Co-Authored-By: Claude Fable 5 --- README.md | 20 +++ src/cli.ts | 3 + src/extract.ts | 233 +++++++++++++++++++++++++++++++++- src/types.ts | 14 ++ src/vite.ts | 9 +- tests/extract-imports.test.ts | 209 ++++++++++++++++++++++++++++++ tests/runtime-parity.test.ts | 15 ++- 7 files changed, 493 insertions(+), 10 deletions(-) create mode 100644 tests/extract-imports.test.ts diff --git a/README.md b/README.md index 1bb8038..bdf7ae4 100644 --- a/README.md +++ b/README.md @@ -350,9 +350,29 @@ solidTranslate({ batchSize: 50, // Keys per API call (default: 50) autoExtract: true, // Auto-extract and msg() strings (default: false) include: ["src/**/*.tsx"], // Files to scan for extraction + extractImportSources: ["@/i18n"], // Extra module specifiers whose msg/ imports count as markers (optional) }) ``` +### What extraction considers a marker + +Extraction only honors `msg()` calls and ``/`` elements whose +identifier actually refers to solid-translate: + +- Imported bindings must come from `"solid-translate"` or an accepted + re-export wrapper. By default any specifier whose final path segment is + `solid-translate` or `i18n` (e.g. `@/i18n`, `../lib/i18n`) is accepted; + set `extractImportSources` (plugin config) / `"extractImportSources"` + (CLI config) to an explicit list to override the `i18n` heuristic + (`"solid-translate"` itself is always accepted). +- Aliased imports work: `import { msg as m } from "solid-translate"` + extracts `m("...")`. +- Locally bound identifiers are never extracted — a callback parameter + named `msg`, a local `const msg = ...`, or a local component named `T` + will not pollute the catalog or emit warnings. +- Identifiers with no binding at all are still treated as markers by name, + so snippet-style sources keep working. + ## CLI For translating locale files, JSON, Markdown, and MDX outside of the Vite build. diff --git a/src/cli.ts b/src/cli.ts index 4dd77c6..b26bf98 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -203,6 +203,7 @@ async function runExtract() { code, relative(root, file), warnings, + { importSources: config.extractImportSources }, ); for (const entry of extracted) { strings[entry.key] = entry.source; @@ -298,6 +299,8 @@ async function runCheck(jsonOutput: boolean) { const entries = extractStringsFromSource( code, relative(root, file), + undefined, + { importSources: config.extractImportSources }, ); for (const entry of entries) { extracted[entry.key] = entry.source; diff --git a/src/extract.ts b/src/extract.ts index 03134ab..3f2f828 100644 --- a/src/extract.ts +++ b/src/extract.ts @@ -17,6 +17,21 @@ export interface ExtractWarning { message: string; } +/** Options controlling how extraction markers are recognized */ +export interface ExtractOptions { + /** + * Module specifiers accepted as sources of the extraction markers + * (`msg`, ``, ``, ...). When a file imports a marker + * identifier, it is only treated as an extraction marker if the import + * comes from one of these specifiers (exact match). `"solid-translate"` + * is always accepted. When omitted, any specifier whose final path + * segment is `solid-translate` or `i18n` (optionally with an extension, + * e.g. `"@/i18n"`, `"../i18n.ts"`) is accepted — covering host-app + * re-export wrappers. + */ + importSources?: string[]; +} + // Minimal structural type for Babel AST nodes — avoids a hard dependency on // @babel/types (we only walk, never construct). interface Node { @@ -65,6 +80,17 @@ interface Node { * whole `` is skipped with a warning. Wrap such values in `` to * make them extractable. * + * Markers are only honored when the identifier actually refers to + * solid-translate: + * - If the file imports the identifier, the import must come from an + * accepted specifier (see {@link ExtractOptions.importSources}); + * aliased imports (`import { msg as m }`) are resolved to their + * imported name. + * - A local binding (function param, `const msg = ...`, a local component + * named `T`, ...) shadows the marker and is never extracted. + * - An identifier with no binding at all is treated as a marker by name + * (backwards compatible with snippet-style sources). + * * Pass a `warnings` array to collect unextractable shapes (dynamic * `msg()` arguments, spread props on ``, non-literal `` * forms, ...). @@ -73,6 +99,7 @@ export function extractStringsFromSource( code: string, filePath: string, warnings?: ExtractWarning[], + options?: ExtractOptions, ): ExtractedString[] { const results: ExtractedString[] = []; const seen = new Set(); @@ -99,13 +126,40 @@ export function extractStringsFromSource( results.push(entry); }; + const bindings = collectModuleBindings(ast); + const shadowStack: Set[] = []; + + /** + * Resolve a local identifier to the marker it refers to, or null when it + * is bound to something other than a solid-translate import. + */ + const resolveMarker = (localName: string): string | null => { + for (let i = shadowStack.length - 1; i >= 0; i--) { + if (shadowStack[i]!.has(localName)) return null; + } + const imported = bindings.imports.get(localName); + if (imported) { + if (!isAcceptedImportSource(imported.source, options?.importSources)) { + return null; + } + return MARKER_NAMES.has(imported.imported) ? imported.imported : null; + } + if (bindings.moduleLocals.has(localName)) return null; + // Unbound identifier: assume ambient marker (backwards compatible) + return MARKER_NAMES.has(localName) ? localName : null; + }; + const visit = (node: Node) => { + const scopeBindings = collectScopeBindings(node); + if (scopeBindings) shadowStack.push(scopeBindings); + if (node.type === "JSXElement") { const name = jsxName(node); - if (name === "T") { + const marker = name ? resolveMarker(name) : null; + if (marker === "T") { const entry = processT(node, filePath, warn); if (entry) push(entry); - } else if (name === "Plural") { + } else if (marker === "Plural") { for (const entry of processPlural(node, filePath, warn)) { push(entry); } @@ -113,18 +167,191 @@ export function extractStringsFromSource( } else if ( node.type === "CallExpression" && node.callee?.type === "Identifier" && - node.callee.name === "msg" + resolveMarker(node.callee.name) === "msg" ) { const entry = processMsg(node, filePath, warn); if (entry) push(entry); } walkChildren(node, visit); + + if (scopeBindings) shadowStack.pop(); }; visit(ast); return results; } +// --------------------------------------------------------------------------- +// Marker binding resolution +// --------------------------------------------------------------------------- + +/** Identifiers solid-translate exports that act as extraction markers */ +const MARKER_NAMES = new Set([ + "msg", + "T", + "Var", + "Num", + "Currency", + "DateTime", + "Plural", +]); + +/** + * Default accepted import specifiers: `solid-translate` itself, or any + * path whose final segment is `solid-translate` or `i18n` (host-app + * re-export wrappers like `@/i18n`, `~/lib/i18n`, `./i18n.ts`). + */ +const DEFAULT_IMPORT_SOURCE_RE = + /(^|\/)(solid-translate|i18n)(\.[cm]?[jt]sx?)?$/; + +function isAcceptedImportSource( + source: string, + importSources?: string[], +): boolean { + if (source === "solid-translate") return true; + if (importSources) return importSources.includes(source); + return DEFAULT_IMPORT_SOURCE_RE.test(source); +} + +interface ModuleBindings { + /** local name → imported name + module specifier */ + imports: Map; + /** Marker-named identifiers declared at module level (non-import) */ + moduleLocals: Set; +} + +/** Collect import bindings and module-level declarations of marker names */ +function collectModuleBindings(ast: Node): ModuleBindings { + const imports = new Map(); + const moduleLocals = new Set(); + + const body: Node[] = ast.program?.body ?? []; + for (const stmt of body) { + if (stmt.type === "ImportDeclaration") { + const source = String(stmt.source?.value ?? ""); + for (const spec of (stmt.specifiers ?? []) as Node[]) { + const local = spec.local?.name; + if (typeof local !== "string") continue; + if (spec.type === "ImportSpecifier") { + const imported = + spec.imported?.type === "Identifier" + ? spec.imported.name + : String(spec.imported?.value ?? ""); + imports.set(local, { imported, source }); + } else { + // Default / namespace imports never bind a marker directly + imports.set(local, { imported: "*", source }); + } + } + } else { + collectDeclaredNames(stmt, moduleLocals); + } + } + + return { imports, moduleLocals }; +} + +/** Collect marker-named identifiers a statement declares (top level) */ +function collectDeclaredNames(stmt: Node, into: Set) { + if ( + stmt.type === "ExportNamedDeclaration" || + stmt.type === "ExportDefaultDeclaration" + ) { + if (stmt.declaration) collectDeclaredNames(stmt.declaration as Node, into); + return; + } + if (stmt.type === "VariableDeclaration") { + for (const decl of (stmt.declarations ?? []) as Node[]) { + if (decl.id) collectPatternNames(decl.id as Node, into); + } + return; + } + if ( + (stmt.type === "FunctionDeclaration" || + stmt.type === "ClassDeclaration" || + stmt.type === "TSEnumDeclaration") && + stmt.id?.type === "Identifier" + ) { + addMarkerName(stmt.id.name, into); + } +} + +/** Collect identifiers bound by a destructuring/param pattern */ +function collectPatternNames(pattern: Node, into: Set) { + switch (pattern.type) { + case "Identifier": + addMarkerName(pattern.name, into); + break; + case "AssignmentPattern": + collectPatternNames(pattern.left as Node, into); + break; + case "RestElement": + collectPatternNames(pattern.argument as Node, into); + break; + case "ObjectPattern": + for (const prop of (pattern.properties ?? []) as Node[]) { + if (prop.type === "ObjectProperty") { + collectPatternNames(prop.value as Node, into); + } else if (prop.type === "RestElement") { + collectPatternNames(prop.argument as Node, into); + } + } + break; + case "ArrayPattern": + for (const el of (pattern.elements ?? []) as (Node | null)[]) { + if (el) collectPatternNames(el, into); + } + break; + } +} + +function addMarkerName(name: unknown, into: Set) { + if (typeof name === "string" && MARKER_NAMES.has(name)) into.add(name); +} + +const FUNCTION_TYPES = new Set([ + "ArrowFunctionExpression", + "FunctionExpression", + "FunctionDeclaration", + "ObjectMethod", + "ClassMethod", + "ClassPrivateMethod", +]); + +/** + * Marker-named identifiers a node binds for its subtree (function params, + * block-level declarations, catch params, for-loop bindings). Returns null + * when the node introduces no relevant bindings — most nodes — so the + * shadow stack stays shallow. + */ +function collectScopeBindings(node: Node): Set | null { + const bound = new Set(); + + if (FUNCTION_TYPES.has(node.type)) { + if (node.id?.type === "Identifier") addMarkerName(node.id.name, bound); + for (const param of (node.params ?? []) as Node[]) { + collectPatternNames(param, bound); + } + } else if (node.type === "CatchClause" && node.param) { + collectPatternNames(node.param as Node, bound); + } else if (node.type === "BlockStatement") { + for (const stmt of (node.body ?? []) as Node[]) { + collectDeclaredNames(stmt, bound); + } + } else if ( + node.type === "ForStatement" || + node.type === "ForOfStatement" || + node.type === "ForInStatement" + ) { + const init = node.init ?? node.left; + if (init?.type === "VariableDeclaration") { + collectDeclaredNames(init as Node, bound); + } + } + + return bound.size > 0 ? bound : null; +} + // --------------------------------------------------------------------------- // AST walking // --------------------------------------------------------------------------- diff --git a/src/types.ts b/src/types.ts index 7f6e1ac..7e54264 100644 --- a/src/types.ts +++ b/src/types.ts @@ -35,6 +35,13 @@ export interface SolidTranslatePluginConfig { * Default: `["src/**\/*.tsx", "src/**\/*.ts", "src/**\/*.jsx"]` */ include?: string[]; + /** + * Module specifiers accepted as sources of the extraction markers + * (`msg`, ``, ``, ...). `"solid-translate"` is always + * accepted. When omitted, any specifier whose final path segment is + * `solid-translate` or `i18n` (e.g. `"@/i18n"`) is accepted. + */ + extractImportSources?: string[]; } /** A flat dictionary mapping keys to translated strings */ @@ -99,4 +106,11 @@ export interface CLIConfig { }; /** Glob patterns for source files to scan */ include?: string[]; + /** + * Module specifiers accepted as sources of the extraction markers + * (`msg`, ``, ``, ...). `"solid-translate"` is always + * accepted. When omitted, any specifier whose final path segment is + * `solid-translate` or `i18n` (e.g. `"@/i18n"`) is accepted. + */ + extractImportSources?: string[]; } diff --git a/src/vite.ts b/src/vite.ts index a830894..500c5d2 100644 --- a/src/vite.ts +++ b/src/vite.ts @@ -51,6 +51,7 @@ export function solidTranslate(config: SolidTranslatePluginConfig): Plugin { translate = true, autoExtract = false, include = ["src/**/*.tsx", "src/**/*.ts", "src/**/*.jsx"], + extractImportSources, } = config; let root: string; @@ -90,7 +91,11 @@ export function solidTranslate(config: SolidTranslatePluginConfig): Plugin { // Auto-extraction: scan source files for and msg() strings let contexts: Record = {}; if (autoExtract) { - const extracted = await autoExtractStrings(root, include); + const extracted = await autoExtractStrings( + root, + include, + extractImportSources, + ); contexts = extracted.contexts; // Merge into source locale file @@ -294,6 +299,7 @@ export default solidTranslate; async function autoExtractStrings( root: string, patterns: string[], + importSources?: string[], ): Promise<{ strings: Record; contexts: Record }> { const strings: Record = {}; const contexts: Record = {}; @@ -311,6 +317,7 @@ async function autoExtractStrings( code, relative(root, file), warnings, + { importSources }, ); for (const entry of extracted) { strings[entry.key] = entry.source; diff --git a/tests/extract-imports.test.ts b/tests/extract-imports.test.ts new file mode 100644 index 0000000..1bea1e8 --- /dev/null +++ b/tests/extract-imports.test.ts @@ -0,0 +1,209 @@ +import { describe, test, expect } from "bun:test"; +import { extractStringsFromSource, type ExtractWarning } from "../src/extract"; + +describe("extractStringsFromSource — import binding resolution", () => { + // ------------------------------------------------------------------------- + // False positives: identifiers that merely share a marker's name + // ------------------------------------------------------------------------- + + test("msg as a callback param is not a marker (no warning, no extraction)", () => { + const code = ` + import { Match, Switch } from "solid-js"; + const el = ( + + + {(msg) =>
{msg()}
} + + + ); + `; + const warnings: ExtractWarning[] = []; + const result = extractStringsFromSource(code, "test.tsx", warnings); + expect(result).toHaveLength(0); + expect(warnings).toHaveLength(0); + }); + + test("msg param calls with string args are not extracted", () => { + const code = ` + const run = (msg) => msg("Not a translation"); + `; + const result = extractStringsFromSource(code, "test.tsx"); + expect(result).toHaveLength(0); + }); + + test("local const msg function is not extracted", () => { + const code = ` + const msg = (s: string) => s.toUpperCase(); + const a = msg("Shout this"); + `; + const warnings: ExtractWarning[] = []; + const result = extractStringsFromSource(code, "test.ts", warnings); + expect(result).toHaveLength(0); + expect(warnings).toHaveLength(0); + }); + + test("msg declared inside a function body is not extracted", () => { + const code = ` + function outer() { + const msg = (s: string) => s; + return msg("Local only"); + } + `; + const result = extractStringsFromSource(code, "test.ts"); + expect(result).toHaveLength(0); + }); + + test("msg imported from an unrelated module is not extracted", () => { + const code = ` + import { msg } from "some-logging-lib"; + const a = msg("Log line"); + `; + const result = extractStringsFromSource(code, "test.ts"); + expect(result).toHaveLength(0); + }); + + test(" bound to a local component is not extracted", () => { + const code = ` + const T = (props: { children: any }) => {props.children}; + const x = Local component text; + `; + const warnings: ExtractWarning[] = []; + const result = extractStringsFromSource(code, "test.tsx", warnings); + expect(result).toHaveLength(0); + expect(warnings).toHaveLength(0); + }); + + test(" imported from another library is not extracted", () => { + const code = ` + import { T } from "some-ui-kit"; + const x = Not ours; + `; + const result = extractStringsFromSource(code, "test.tsx"); + expect(result).toHaveLength(0); + }); + + test(" bound to a local component is not extracted", () => { + const code = ` + function Plural(props: any) { return null; } + const x = ; + `; + const result = extractStringsFromSource(code, "test.tsx"); + expect(result).toHaveLength(0); + }); + + // ------------------------------------------------------------------------- + // True positives: real solid-translate bindings, aliased and re-exported + // ------------------------------------------------------------------------- + + test("msg imported from solid-translate is extracted", () => { + const code = ` + import { msg } from "solid-translate"; + const a = msg("Save changes"); + `; + const result = extractStringsFromSource(code, "test.ts"); + expect(result).toHaveLength(1); + expect(result[0]!.key).toBe("Save changes"); + }); + + test("aliased import `import { msg as m }` is extracted", () => { + const code = ` + import { msg as m } from "solid-translate"; + const a = m("Aliased marker"); + `; + const result = extractStringsFromSource(code, "test.ts"); + expect(result).toHaveLength(1); + expect(result[0]!.key).toBe("Aliased marker"); + }); + + test("re-export wrapper `import { msg } from \"@/i18n\"` is extracted", () => { + const code = ` + import { msg } from "@/i18n"; + const a = msg("From re-export"); + `; + const result = extractStringsFromSource(code, "test.ts"); + expect(result).toHaveLength(1); + expect(result[0]!.key).toBe("From re-export"); + }); + + test("relative i18n re-export paths are accepted by default", () => { + const code = ` + import { msg } from "../lib/i18n"; + const a = msg("Relative re-export"); + `; + const result = extractStringsFromSource(code, "test.ts"); + expect(result).toHaveLength(1); + expect(result[0]!.key).toBe("Relative re-export"); + }); + + test("aliased import is extracted", () => { + const code = ` + import { T as Trans } from "solid-translate"; + const x = Aliased element; + `; + const result = extractStringsFromSource(code, "test.tsx"); + expect(result).toHaveLength(1); + expect(result[0]!.key).toBe("Aliased element"); + }); + + test("shadowing only applies inside the shadowing scope", () => { + const code = ` + import { msg } from "solid-translate"; + const outer = msg("Outer real"); + const inner = (msg: (s: string) => string) => msg("Inner shadowed"); + const after = msg("After real"); + `; + const result = extractStringsFromSource(code, "test.ts"); + expect(result.map((r) => r.key)).toEqual(["Outer real", "After real"]); + }); + + // ------------------------------------------------------------------------- + // Backwards compatibility: unbound identifiers stay markers + // ------------------------------------------------------------------------- + + test("unbound and msg() are still extracted (snippet sources)", () => { + const code = ` + const x = Ambient element; + const a = msg("Ambient call"); + `; + const result = extractStringsFromSource(code, "test.tsx"); + expect(result.map((r) => r.key)).toEqual(["Ambient element", "Ambient call"]); + }); + + // ------------------------------------------------------------------------- + // Configurable accepted sources + // ------------------------------------------------------------------------- + + test("importSources option accepts custom specifiers", () => { + const code = ` + import { msg } from "#translate"; + const a = msg("Custom wrapper"); + `; + const without = extractStringsFromSource(code, "test.ts"); + expect(without).toHaveLength(0); + + const withOption = extractStringsFromSource(code, "test.ts", undefined, { + importSources: ["#translate"], + }); + expect(withOption).toHaveLength(1); + expect(withOption[0]!.key).toBe("Custom wrapper"); + }); + + test("importSources option replaces the default i18n heuristic but keeps solid-translate", () => { + const code = ` + import { msg } from "@/i18n"; + const a = msg("Heuristic path"); + `; + const result = extractStringsFromSource(code, "test.ts", undefined, { + importSources: ["#translate"], + }); + expect(result).toHaveLength(0); + + const direct = extractStringsFromSource( + `import { msg } from "solid-translate"; const a = msg("Always ok");`, + "test.ts", + undefined, + { importSources: ["#translate"] }, + ); + expect(direct).toHaveLength(1); + }); +}); diff --git a/tests/runtime-parity.test.ts b/tests/runtime-parity.test.ts index 91b0e09..ccdb4ef 100644 --- a/tests/runtime-parity.test.ts +++ b/tests/runtime-parity.test.ts @@ -58,9 +58,12 @@ export default function Fixture(props) { } `; - const keys = extractStringsFromSource(source, "fixture.tsx").map( - (e) => e.key, - ); + // The fixture imports the markers from the in-repo source entry point, + // so declare it as an accepted import source (a consumer app would + // import from "solid-translate" or an i18n re-export instead). + const keys = extractStringsFromSource(source, "fixture.tsx", undefined, { + importSources: ["../../src/index.ts"], + }).map((e) => e.key); const compiled = await transformAsync(source, { presets: [[presetSolid, { generate: "dom", hydratable: false }]], @@ -345,9 +348,9 @@ export default function Fixture(props) { ); } `; - const keys = extractStringsFromSource(source, "fixture.tsx").map( - (e) => e.key, - ); + const keys = extractStringsFromSource(source, "fixture.tsx", undefined, { + importSources: ["../../src/index.ts"], + }).map((e) => e.key); expect(keys).toContain("Good morning"); const compiled = await transformAsync(source, {