Angular: Fix single-quoted literal union types not inferring select control#34902
Angular: Fix single-quoted literal union types not inferring select control#34902asiokun wants to merge 1 commit into
Conversation
…ontrol Compodoc emits union types with single-quoted string literals (e.g. 'S' | 'M' | 'L') that previously failed JSON.parse, causing extractEnumValues to return null and falling back to an object control instead of select/radio. Fix: parse each segment individually, stripping surrounding single or double quotes before returning the literal string value. Keeps JSON.parse fallback for non-string literals (numbers, booleans, null). Also strengthens the enum childs check with optional chaining and adds regression tests. Closes storybookjs#12641 Closes storybookjs#33779
|
📝 WalkthroughWalkthroughThis PR enhances the Angular compodoc type extraction to better handle enum union types with single-quoted literals. The ChangesEnum Union Type Parsing
🎯 2 (Simple) | ⏱️ ~10 minutes 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.
🧹 Nitpick comments (1)
code/frameworks/angular/src/client/compodoc.ts (1)
133-148: 💤 Low valueConsider handling escaped quotes within string literals.
The current quote-stripping logic handles simple cases well but doesn't account for escaped quotes within the string (e.g.,
'it\'s'or"say \"hi\""). If compodoc ever emits such values,slice(1, -1)would leave the backslash-escaped quote intact rather than unescaping it.This is likely a non-issue in practice since TypeScript union literals rarely contain escaped quotes, but worth noting for completeness.
🤖 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 `@code/frameworks/angular/src/client/compodoc.ts` around lines 133 - 148, The current quoted-string branch in the compodoc parser (inside the compodocType -> segments -> parsed mapping) naively does slice(1,-1) which leaves backslash-escaped quotes intact; update that branch to remove the outer quotes then unescape escape sequences so `'it\'s'` and `"say \"hi\""` become its and say "hi". Concretely: detect the quote char, strip it, then create a normalized double-quoted string (escape any internal double quotes) and run JSON.parse on it to let the JSON unescaping handle backslashes; keep the existing fallback JSON.parse/return-segment logic for non-quoted segments.
🤖 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.
Nitpick comments:
In `@code/frameworks/angular/src/client/compodoc.ts`:
- Around line 133-148: The current quoted-string branch in the compodoc parser
(inside the compodocType -> segments -> parsed mapping) naively does slice(1,-1)
which leaves backslash-escaped quotes intact; update that branch to remove the
outer quotes then unescape escape sequences so `'it\'s'` and `"say \"hi\""`
become its and say "hi". Concretely: detect the quote char, strip it, then
create a normalized double-quoted string (escape any internal double quotes) and
run JSON.parse on it to let the JSON unescaping handle backslashes; keep the
existing fallback JSON.parse/return-segment logic for non-quoted segments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8f772c40-60d4-4ae0-af5e-dd69ec11f416
📒 Files selected for processing (2)
code/frameworks/angular/src/client/compodoc.test.tscode/frameworks/angular/src/client/compodoc.ts
|
Superseded by #34887 |
Problem
Fixes #12641, Fixes #33779
Angular's Compodoc emits TypeScript literal union types using single-quoted strings (e.g.
'S' | 'M' | 'L'). The previousextractEnumValuesimplementation calledJSON.parse()on each segment without pre-processing, but single-quoted strings are not valid JSON — causing the entire parse to throw and fall back tonull, resulting in anobjectcontrol instead of the expectedselect/radio.Double-quoted unions like
"primary" | "secondary"already worked correctly.Root Cause
Fix
Parse each pipe-delimited segment individually:
JSON.parsefor non-string literals (numbers, booleans,null)JSON.parsealso failsAlso strengthens the enum
childsguard with optional chaining (?.value != nullinstead of.valuetruthiness, which excluded the string"0").Regression tests added
Two new test cases in
compodoc.test.ts:"'primary' | 'secondary'"→{ name: 'enum', value: ['primary', 'secondary'] }(single-quoted, 2 items → radio)"'S' | 'M' | 'L'"→{ name: 'enum', value: ['S', 'M', 'L'] }(single-quoted, 3 items → radio)The existing
'"primary" | "secondary"'test (double-quoted) continues to pass.Manual Testing
With Angular
input<'S' | 'M' | 'L'>()or@Input() size: 'S' | 'M' | 'L':objectinput boxBounty: $110 on Opire.dev
Summary by CodeRabbit
Tests
Bug Fixes
0,false).