Skip to content

fix(types): correct intersection, indexedAccess, and add predicate/templateLiteral#66

Open
sujalgoel wants to merge 3 commits intowebpack:mainfrom
sujalgoel:fix/types-mjs-correctness
Open

fix(types): correct intersection, indexedAccess, and add predicate/templateLiteral#66
sujalgoel wants to merge 3 commits intowebpack:mainfrom
sujalgoel:fix/types-mjs-correctness

Conversation

@sujalgoel
Copy link
Copy Markdown
Contributor

Problem

Four bugs in plugins/theme/partials/types.mjs:

1. intersection rendered with | instead of &

The union and intersection cases shared one return union(type.types) call. union() defaults to |, so A & B was rendered as {A|B} — same as a union type.

2. indexedAccess lost the index type

optional and indexedAccess were merged under one case using type.elementType ?? type.objectType. IndexedAccessType has no elementType field, so the fallback always resolved to objectType alone — dropping the index type entirely.

  • T[K] was rendered as {T}, now renders as {T[K]}

3. Missing predicate case

TypeDoc emits predicate types for type guard signatures like x is string and asserts x is string. These fell through to the default branch and returned type.name ?? 'unknown' — dropping the target type.

4. Missing templateLiteral case

Template literal types (`prefix_${string}`) also fell through to the default branch.

Fix

  • Split union and intersection into separate cases; intersection passes '&' as the separator
  • Split optional and indexedAccess into separate cases; indexedAccess renders as objectType[indexType]
  • Add predicate case: (asserts?) name (is targetType?)
  • Add templateLiteral case: `head${type1}span1${type2}span2...`

Verification

Pipeline (node generate-md.mjs) runs clean. Spot-checked output:

// Before
> **AssetInfo** = {KnownAssetInfo|Record<string, any>}

// After  
> **AssetInfo** = {KnownAssetInfo&Record<string, any>}

…mplateLiteral

- `intersection` was using `|` separator (same as union); now correctly uses `&`
  - e.g. `A & B` previously rendered as `{A|B}`, now `{A&B}`
- `indexedAccess` and `optional` were merged under one case using
  `type.elementType ?? type.objectType`; `IndexedAccessType` has no
  `elementType` field, so the fallback always lost the index type entirely
  - e.g. `T[K]` previously rendered as `{T}`, now `{T[K]}`
- Add `predicate` case for type predicate signatures (`x is T`,
  `asserts x is T`)
- Add `templateLiteral` case for template literal types
  (e.g. `` `prefix_${string}` ``)
Copy link
Copy Markdown
Member

@avivkeller avivkeller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doc-kit doesn't yet support this. Please very that before sending a PR

@sujalgoel
Copy link
Copy Markdown
Contributor Author

Checked this against doc-kit's normalizeTypes regex.

templateLiteral is the actual problem. ${...} puts { and } inside the type annotation, both excluded by [^<({})>], so {prefix_${string}} matches as {string} — wrong type, wrong link.

The other three are fine:

{A&B}          → matches (intersection fix)
{T[K]}         → matches (indexedAccess fix)
{x is string}  → matches (predicate)

Neither predicate nor templateLiteral show up in webpack's types.d.ts right now, so both are theoretical anyway. The intersection and indexedAccess fixes are for types already in the generated output: AssetInfo, getIncomingConnectionsByOriginModule, a few others.

Happy to either drop templateLiteral and ship the other three, or split the PR into two: bugs now, new type cases once doc-kit has support. Whatever works better for you.

@sujalgoel sujalgoel requested a review from avivkeller March 29, 2026 19:21
@avivkeller
Copy link
Copy Markdown
Member

Just because the normalizing regex matches the content of the {...}, doesn't mean they will be properly mapped to types.

Comment on lines +39 to +42
case 'predicate': {
const target = type.targetType ? ` is ${resolve(type.targetType)}` : '';
return (type.asserts ? 'asserts ' : '') + type.name + target;
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should just return the target, perhaps

Comment on lines +44 to +50
case 'templateLiteral':
return (
'`' +
type.head +
type.tail.map(([t, s]) => '${' + resolve(t) + '}' + s).join('') +
'`'
);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert

@sujalgoel
Copy link
Copy Markdown
Contributor Author

Got it. The theme fix needs doc-kit to support & and indexed access first. Is there a tracking issue for this, or should I open one?

Per review feedback:
- predicate: return resolve(type.targetType) instead of full predicate syntax
- templateLiteral: reverted, doc-kit does not yet support this
@sujalgoel sujalgoel requested a review from avivkeller March 29, 2026 20:07
@avivkeller
Copy link
Copy Markdown
Member

Neither, if you'd like to open a PR to doc-kit, please do. A tracking issue isn't really needed, since it's more of a QoL thing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants