Populate attribute name in flow error messages - #68
Conversation
Flow error messages resolved from a translation bundle kept the backend's
`{{param(name)}}` placeholders literally, because `t()` only substituted the
`{name}` syntax used by the SDK's own bundles and `extractErrorMessage` never
passed `error.message.params` through. The server ships the whole `system`
namespace via flow meta, so the bundle lookup always won over the
already-substituted `defaultValue`, and an attribute uniqueness failure
rendered as "User already exists with the provided {{param(attribute)}}".
Add a shared `substituteTranslationParams` util that handles both placeholder
syntaxes, use it in all three `t()` implementations, and pass the error params
into the lookups. A translation that still holds an unresolved placeholder is
now treated as a miss so resolution falls back to `defaultValue`.
Fixes thunder-id/thunderid#4796
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughChangesTranslation parameter handling
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/javascript/src/index.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. packages/javascript/src/models/embedded-flow.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. packages/javascript/src/utils/__tests__/substituteTranslationParams.test.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/react/src/utils/__tests__/flowTransformer.test.ts`:
- Around line 4-29: Replace the local createTranslator parameter-substitution
implementations in packages/react/src/utils/__tests__/flowTransformer.test.ts
(lines 4-29) and packages/vue/src/utils/__tests__/flowTransformer.test.ts (lines
4-29) with substituteTranslationParams from `@thunderid/browser`. Preserve each
test translator’s existing bundle lookup and missing-key behavior while
delegating parameter substitution to the shared helper in both files.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 290210a4-961d-42bf-bc2f-f0e831e7278e
📒 Files selected for processing (11)
packages/javascript/src/index.tspackages/javascript/src/models/embedded-flow.tspackages/javascript/src/utils/__tests__/substituteTranslationParams.test.tspackages/javascript/src/utils/substituteTranslationParams.tspackages/react/src/contexts/I18n/I18nProvider.tsxpackages/react/src/hooks/useTranslation.tspackages/react/src/utils/__tests__/flowTransformer.test.tspackages/react/src/utils/flowTransformer.tspackages/vue/src/providers/I18nProvider.tspackages/vue/src/utils/__tests__/flowTransformer.test.tspackages/vue/src/utils/flowTransformer.ts
| import {describe, it, expect} from 'vitest'; | ||
| import {extractErrorMessage} from '../flowTransformer'; | ||
|
|
||
| const UNIQUENESS_KEY = 'flows.executor.errors.attribute_not_unique'; | ||
|
|
||
| /** | ||
| * Builds a `t` stub backed by a flat bundle, mirroring how `I18nProvider` resolves keys: | ||
| * a miss returns the key itself, and params are substituted into the resolved value. | ||
| */ | ||
| const createTranslator = | ||
| (bundle: Record<string, string> = {}) => | ||
| (key: string, params?: Record<string, string | number>): string => { | ||
| const translation: string = bundle[key] ?? key; | ||
|
|
||
| if (!params) { | ||
| return translation; | ||
| } | ||
|
|
||
| return Object.entries(params).reduce( | ||
| (acc: string, [paramKey, paramValue]: [string, string | number]): string => | ||
| acc | ||
| .replace(new RegExp(`\\{\\{\\s*param\\(\\s*${paramKey}\\s*\\)\\s*\\}\\}`, 'g'), String(paramValue)) | ||
| .replace(new RegExp(`\\{${paramKey}\\}`, 'g'), String(paramValue)), | ||
| translation, | ||
| ); | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify that the shared helper is exported and find the duplicate test fixtures.
rg -n -C 3 'substituteTranslationParams|const createTranslator' packagesRepository: thunder-id/javascript-sdks
Length of output: 13090
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- shared helper ---'
cat -n packages/javascript/src/utils/substituteTranslationParams.ts
printf '%s\n' '--- browser exports ---'
rg -n -C 3 'substituteTranslationParams' packages/browser/src packages/javascript/src/index.ts
printf '%s\n' '--- React fixture ---'
cat -n packages/react/src/utils/__tests__/flowTransformer.test.ts | sed -n '1,35p'
printf '%s\n' '--- Vue fixture ---'
cat -n packages/vue/src/utils/__tests__/flowTransformer.test.ts | sed -n '1,35p'
printf '%s\n' '--- package dependency declarations ---'
rg -n -C 2 '"`@thunderid/`(javascript|browser)"' packages/react/package.json packages/vue/package.jsonRepository: thunder-id/javascript-sdks
Length of output: 7403
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import re
def local_substitute(translation, params):
for param_key, param_value in params.items():
translation = re.sub(
r'\{\{\s*param\(\s*' + param_key + r'\s*\)\s*\}\}',
lambda m: str(param_value),
translation,
)
translation = re.sub(
r'\{' + param_key + r'\}',
lambda m: str(param_value),
translation,
)
return translation
def shared_substitute(translation, params):
if not translation or not params:
return translation
for param_key, param_value in params.items():
escaped = re.escape(param_key)
translation = re.sub(
r'\{\{\s*param\(\s*' + escaped + r'\s*\)\s*\}\}',
lambda m: str(param_value),
translation,
)
translation = re.sub(
r'\{' + escaped + r'\}',
lambda m: str(param_value),
translation,
)
return translation
cases = [
("Value: {{param(attribute)}}", {"attribute": "$&"}),
("Value: {a.b}", {"a.b": "ok"}),
("Value: {a+b}", {"a+b": "ok"}),
]
for translation, params in cases:
print(repr((translation, params)))
print("local :", repr(local_substitute(translation, params)))
print("shared:", repr(shared_substitute(translation, params)))
PYRepository: thunder-id/javascript-sdks
Length of output: 404
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- browser package exports ---'
fd -i 'index.ts' packages/browser packages/javascript | sort | xargs -r rg -n -C 4 'substituteTranslationParams|`@thunderid/javascript`'
printf '%s\n' '--- JavaScript replacement semantics ---'
node - <<'JS'
function localSubstitute(translation, params) {
return Object.entries(params).reduce(
(acc, [paramKey, paramValue]) =>
acc
.replace(new RegExp(`\\{\\{\\s*param\\(\\s*${paramKey}\\s*\\)\\s*\\}\\}`, 'g'), String(paramValue))
.replace(new RegExp(`\\{${paramKey}\\}`, 'g'), String(paramValue)),
translation,
);
}
function escapeRegExp(value) {
return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
}
function sharedSubstitute(translation, params) {
return Object.entries(params).reduce((acc, [paramKey, paramValue]) => {
const escapedKey = escapeRegExp(paramKey);
const value = String(paramValue);
return acc
.replace(new RegExp(`\\{\\{\\s*param\\(\\s*${escapedKey}\\s*\\)\\s*\\}\\}`, 'g'), () => value)
.replace(new RegExp(`\\{${escapedKey}\\}`, 'g'), () => value);
}, translation);
}
for (const [translation, params] of [
['Value: {{param(attribute)}}', {attribute: '$&'}],
['Value: {{param(a+b)}}', {'a+b': 'ok'}],
['Value: {a.b}', {'a.b': 'ok'}],
]) {
console.log(JSON.stringify({translation, params}));
console.log('local :', JSON.stringify(localSubstitute(translation, params)));
console.log('shared:', JSON.stringify(sharedSubstitute(translation, params)));
}
JSRepository: thunder-id/javascript-sdks
Length of output: 1988
Use substituteTranslationParams from @thunderid/browser in both test translators.
The local implementations do not escape parameter names and pass replacement values as strings. Values such as a+b or $& therefore differ from provider behavior in both files.
🧰 Tools
🪛 ast-grep (0.45.1)
[warning] 24-24: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(\\{\\{\\s*param\\(\\s*${paramKey}\\s*\\)\\s*\\}\\}, 'g')
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
[warning] 25-25: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(\\{${paramKey}\\}, 'g')
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
📍 Affects 2 files
packages/react/src/utils/__tests__/flowTransformer.test.ts#L4-L29(this comment)packages/vue/src/utils/__tests__/flowTransformer.test.ts#L4-L29
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/react/src/utils/__tests__/flowTransformer.test.ts` around lines 4 -
29, Replace the local createTranslator parameter-substitution implementations in
packages/react/src/utils/__tests__/flowTransformer.test.ts (lines 4-29) and
packages/vue/src/utils/__tests__/flowTransformer.test.ts (lines 4-29) with
substituteTranslationParams from `@thunderid/browser`. Preserve each test
translator’s existing bundle lookup and missing-key behavior while delegating
parameter substitution to the shared helper in both files.
Source: Coding guidelines
2 participants
Purpose
This pull request introduces an unified approach to parameter substitution in i18n translation strings across the JavaScript, React, and Vue SDKs. It adds a utility for substituting parameters in both backend (
{{param(name)}}) and bundle ({name}) placeholder syntaxes, updates all translation hooks and providers to use it, and refines error message extraction to ensure fully resolved, user-presentable messages. Comprehensive tests are included to validate the new logic.Approach
Parameter substitution improvements:
substituteTranslationParams, which handles both backend ({{param(name)}}) and bundle ({name}) parameter syntaxes, and a helperhasUnresolvedTranslationParamsto detect unresolved placeholders. Exported these from the main SDK entry point. [1] [2]I18nMessageinterface to clarify thatdefaultValueis pre-substituted by the backend, while bundle lookups require substitution withparams.SDK integration (React & Vue):
I18nProvider.tsx,useTranslation.ts) and Vue (I18nProvider.ts) to use the newsubstituteTranslationParamsutility, replacing ad-hoc parameter substitution code. [1] [2] [3] [4] [5] [6]Error message extraction logic:
flowTransformer.ts(React and Vue) to:paramsto translation lookups.hasUnresolvedTranslationParamsto fall back todefaultValueif any placeholders remain after substitution, ensuring only fully-resolved messages are shown to users.I18nMessagetype for error messages and descriptions. [1] [2] [3] [4] [5]Testing:
These changes ensure consistent, correct, and maintainable parameter substitution for all i18n messages and error handling across the SDKs.
Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit
New Features
Bug Fixes
Tests