fix(js): add fallback enum key when unicode values produce empty identifier#1358
fix(js): add fallback enum key when unicode values produce empty identifier#1358Adwaith-Jayan wants to merge 2 commits intoappwrite:masterfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughIntroduces an intermediate processed key variable in the enum template and uses it to derive enum member names; when the processed key is empty, the template falls back to a deterministic name of the form Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
templates/web/src/enums/enum.ts.twig (1)
4-4: Apply whitespace-control modifiers to prevent extra blank lines between enum membersThe Twig environment in
src/SDK/SDK.phpdoes not enabletrim_blocksorlstrip_blocks, so each{% ... %}tag will leave a trailing newline. This means adding line 4 introduces an extra blank line per enum member. The deno template already uses the{%~modifier (e.g., line 2-3) as a best practice. Apply the same approach:Suggested change
- {% set processed = key | replace({'-': ''}) | caseEnumKey %} + {%- set processed = key | replace({'-': ''}) | caseEnumKey -%}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates/web/src/enums/enum.ts.twig` at line 4, The Twig "{% set processed = key | replace({'-': ''}) | caseEnumKey %}" tag emits a trailing newline; update the tag in enum.ts.twig to use Twig's whitespace-control modifier (~) on the tag so the set tag does not produce an extra blank line per enum member (target the "{% set processed ... %}" tag that defines the processed variable and apply the '~' whitespace control to the tag).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@templates/web/src/enums/enum.ts.twig`:
- Around line 4-5: The fallback identifier for empty processed keys uses "Value"
+ loop.index0 which can collide with a legitimate processed key; update the
template logic so the fallback uses an unlikely prefix (e.g., "_Value" or
"UniqueValue") instead of "Value" (modify where processed is computed and the
ternary that emits {{ processed is empty ? 'Value' ~ loop.index0 : processed
}}), or implement conflict detection/resolution in the code that prepares the
keys (ensuring caseEnumKey-normalized names plus fallbacks are unique before
rendering).
- Line 5: The enum template emits raw single-quoted values which break
TypeScript when the value contains a single quote; update the template line in
templates/web/src/enums/enum.ts.twig to output a properly escaped/quoted string
literal (e.g. use Twig's json_encode or explicit replace) instead of raw '{{
value }}'. Concretely, replace the current "= '{{ value }}'," with a safe
expression such as "= {{ value|json_encode|raw }}," (or "= '{{
value|replace(\"'\",\"\\\\'\") }}'," if you prefer single quotes) so values like
"it's" are emitted as valid TypeScript string literals; keep the surrounding
identifier logic (processed / loop.index0) unchanged.
---
Nitpick comments:
In `@templates/web/src/enums/enum.ts.twig`:
- Line 4: The Twig "{% set processed = key | replace({'-': ''}) | caseEnumKey
%}" tag emits a trailing newline; update the tag in enum.ts.twig to use Twig's
whitespace-control modifier (~) on the tag so the set tag does not produce an
extra blank line per enum member (target the "{% set processed ... %}" tag that
defines the processed variable and apply the '~' whitespace control to the tag).
… unicode values
What does this PR do?
Fixes enum generation for the TypeScript SDK when enum values contain non-English (Unicode) characters.
Currently, when enum values contain characters outside
[a-zA-Z0-9], thetoCamelCase()transformation strips all characters, resulting in an empty enum identifier and invalid TypeScript output.Example of current broken output:
Test Plan
toCamelCase()transformation removing non[a-zA-Z0-9]characters.templates/web/src/enums/enum.ts.twigto introduce a fallback when the processed enum key is empty.composer lint-twigto ensure there are no Twig syntax errors.Value0,Value1, etc.).Related PRs and Issues
Closes #1256
Have you read the Contributing Guidelines on issues?
Yes.
Summary by CodeRabbit