element: Make createInterpolateElement TS/type smart#71513
Merged
manzoorwanijk merged 9 commits intotrunkfrom Apr 1, 2026
Merged
element: Make createInterpolateElement TS/type smart#71513manzoorwanijk merged 9 commits intotrunkfrom
manzoorwanijk merged 9 commits intotrunkfrom
Conversation
manzoorwanijk
added
the
[Type] Code Quality
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
manzoorwanijk
changed the title
|
Size Change: -1 B (0%) Total Size: 7.73 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in e4ae743. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/23811579637
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enhances the createInterpolateElement function with TypeScript type safety by leveraging template literal types to extract tag names from the interpolated string and enforce type-checking on the conversion map.
Key changes:
- New type definitions that extract and validate tag names from template literal strings
- Enhanced function signature with generic type parameters for type-safe conversion maps
- Updated tests with
@ts-expect-errorannotations to verify type checking works correctly
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/element/src/types.ts | Introduces TypeScript utility types for extracting tags from strings and creating type-safe conversion maps |
| packages/element/src/create-interpolate-element.ts | Updates function signature to use generic types for enhanced type safety |
| packages/element/src/test/create-interpolate-element.tsx | Adds @ts-expect-error comments to verify type checking and fixes TestComponent render method |
| packages/element/README.md | Updates parameter documentation to reflect new type signatures |
Comments suppressed due to low confidence (1)
packages/element/src/test/create-interpolate-element.tsx:169
- Fixed: The render method in React class components doesn't receive props as a parameter. The correct approach is to access props via 'this.props', which this change implements correctly.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
manzoorwanijk
force-pushed
the
update/make-create-interpolate-element-smart
branch
2 times, most recently
from
bf188ec to
dff0669
Compare
manzoorwanijk
force-pushed
the
update/make-create-interpolate-element-smart
branch
from
dff0669 to
2f8715f
Compare
SirLouen
reviewed
Jan 18, 2026
manzoorwanijk
commented
Jan 18, 2026
jsnajdr
reviewed
Mar 31, 2026
Member
jsnajdr
left a comment
jsnajdr
left a comment
There was a problem hiding this comment.
Hi @manzoorwanijk 👋 What if we finished this PR? 🙂 I posted some review comments.
Member
There was a problem hiding this comment.
The item tag is not part of the inferred conversion map type, is that expected? Only somethingElse is detected, and the map type is Partial<Record<"somethingElse">>.
Member
There was a problem hiding this comment.
One additional unit test that would be nice to have is where we test explicitly if ExtractTags really extracts all the tags. For example, from <a><b><c>, the type should be "a"|"b"|"c". That would test the template matching algorithm.
| */ | ||
| type TranslatableText< T extends string > = string & { | ||
| readonly __translatableText: T; | ||
| }; |
Member
There was a problem hiding this comment.
Can we have an unit test for a TranslatableText argument?
My understanding is that we need this special case in order to extract the "real" string from the right place. Not from the actual string itself, but from its __translatableText field.
Member
Author
There was a problem hiding this comment.
Added a type-level test that verifies InterpolationString correctly unwraps a TranslatableText input, and ExtractTags then extracts the tags from the underlying literal type.
packages/element/src/types.ts
Outdated
| T extends `${ string }<${ infer Tag }>${ infer After }` | ||
| ? ExtractTagName< Tag > extends never | ||
| ? ExtractTags< After > | ||
| : ExtractTagName< Tag > | ExtractTags< After > |
Member
There was a problem hiding this comment.
One of the extends never check is redundant. We can do:
T extends `${string}<${infer T}>${infer A}`
? ExtractTagName<T> | ExtractTags<A>
: neverThat's because never|T is always equivalent to T, never is the TypeScript "bottom type".
Member
Author
There was a problem hiding this comment.
Good call — simplified.
The `extends never` branch is unnecessary because `never | T` is always equivalent to `T` — `never` is TypeScript's bottom type.
The runtime tokenizer regex `/<(\/)?(\w+)\s*(\/)?>/g` allows optional whitespace between the tag name and the closing slash (e.g. `<item />`). The type-level ExtractTagName was incorrectly filtering these as "tags with spaces". Reorder the checks so `<name />` is recognized as a valid self-closing tag before the general space filter applies.
Verify that ExtractTags correctly extracts all tag names from a template literal string, and that InterpolationString properly unwraps TranslatableText to extract the underlying literal type.
manzoorwanijk
force-pushed
the
update/make-create-interpolate-element-smart
branch
from
349facf to
19473da
Compare
Member
Author
|
@jsnajdr I have addressed your feedback. |
jsnajdr
reviewed
Mar 31, 2026
packages/element/src/types.ts
Outdated
| */ | ||
| type ExtractTagName< T extends string > = T extends `/${ string }` | ||
| ? never // Skip closing tags like "/div" | ||
| : T extends `${ infer Name } /` |
Member
There was a problem hiding this comment.
This will fail again when there are two spaces. ChatGPT proposed a recursive helper:
type TrimRightSpaces<S extends string> =
S extends `${infer Rest} `
? TrimRightSpaces<Rest>
: S;
type ExtractTagName<S extends string> =
// forbid leading space
S extends ` ${string}`
? never
: TrimRightSpaces<S> extends infer Name extends string
// optional: reject empty tag
? Name extends ""
? never
// forbid inner spaces after trailing spaces were removed
: Name extends `${string} ${string}`
? never
: Name
: never;This could be used. There is also a nice trick with double extends that helps prevent two calls of TrimRightSpaces:
TrimRightSpaces<S> extends infer Name extends string
Member
Author
There was a problem hiding this comment.
Good catch! Updated.
Replace the single-space pattern match with a recursive TrimTrailingSpaces helper that handles any number of trailing spaces before the self-closing slash, matching the runtime tokenizer's \s* behavior.
jsnajdr
reviewed
Apr 1, 2026
| export type ExtractTags< T extends string > = | ||
| T extends `${ string }<${ infer Tag }>${ infer After }` | ||
| ? ExtractTagName< Tag > | ExtractTags< After > | ||
| : never; |
Member
There was a problem hiding this comment.
There's one more little bug that I don't know how to solve. Consider this call where the input is a generic string and the conversion map has invalid values (number instead of element):
createInterpolateElement( foo, { a: 1 } )There will be no type error, although the value of a should be reported as wrong. The reason is that for string input, conversion map type is Record<never, ReactElement>. Because of the never key, this is effectively {}, and the a field is considered an "extra" field that is not typechecked.
A better type would be Record<string, ReactElement>. This can be achieved by selectively returning string or never from ExtractTags:
type ExtractTags<T extends string, R = string> =
T extends `tag-pattern`
? ExtractTagName<Tag> | ExtractTags<After, never>
: R;This returns string when T is a generic string, but nested calls will return never.
But this is not satisfactory because then the following call:
createInterpolateElement( 'foo', { a: <em /> } )will stop detecting that a is not a valid key. The foo literal also doesn't match the tag-pattern, just like the generic string. We need to be able to distinguish between a generic string and a literal value.
jsnajdr
approved these changes
Apr 1, 2026
Member
jsnajdr
left a comment
jsnajdr
left a comment
There was a problem hiding this comment.
I think this is ready to go 👍
One more thing that would be nice to solve in a follow-up. Currently the createInterpolateElement types are much less useful than they could be, because so many calls use a nested sprintf:
createInterpolateElement( sprintf( '<Name>%1$s</Name>', name ), { Name } );The return type of sprintf is string, and the smart types infer nothing.
We could use a similar trick as in TranslatableText. The __( 'string to translate' ) helper has the same problem, the result is a generic string, but we can pass through the original literal as the __translatableText virtual field. The sprintf return value could be a similar type.
Member
Author
|
I will create a follow up for |
ObliviousHarmony
pushed a commit
that referenced
this pull request
Apr 1, 2026
Co-authored-by: manzoorwanijk <manzoorwanijk@git.wordpress.org> Co-authored-by: SirLouen <sirlouen@git.wordpress.org> Co-authored-by: jsnajdr <jsnajdr@git.wordpress.org>
adamsilverstein
pushed a commit
that referenced
this pull request
Apr 7, 2026
Co-authored-by: manzoorwanijk <manzoorwanijk@git.wordpress.org> Co-authored-by: SirLouen <sirlouen@git.wordpress.org> Co-authored-by: jsnajdr <jsnajdr@git.wordpress.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
This PR makes
createInterpolateElementtype/TS smart by interpolating the string passed as the first argument and then providing type-checking for theconversionMapprovided as the second argumentWhy?
To enhance type-safety and allow auto-completions for
createInterpolateElementHow?
The PR uses some advanced TypeScrip string interpolation to create types like
ConversionMapwhich uses another typeExtractTagswhich extracts tags from the passed string. The types are commented enough to make them readable and maintainableTesting Instructions
Testing Instructions for Keyboard
Screenshots or screencast