fix(next/image): annotate Image export so deprecated props show as deprecated - #98597
fix(next/image): annotate Image export so deprecated props show as deprecated#98597franpeza wants to merge 1 commit into
Conversation
…es call sites `Image` was declared as `forwardRef<HTMLImageElement | null, ImageProps>(...)` with the return type left to inference. Because `ImageProps` is declared in another module, declaration emit expands the alias inline and drops the JSDoc of every member, so the `@deprecated` tags never reach editors at the call site. Annotating the export explicitly keeps the alias by reference, which preserves the tags for `priority`, `onLoadingComplete`, `layout`, `objectFit`, `objectPosition`, `lazyBoundary` and `lazyRoot`. The emitted type is unchanged in substance: the previous expanded form and `ImageProps & React.RefAttributes<HTMLImageElement | null>` are mutually assignable, and prop completion is identical.
SapanMozammel
left a comment
There was a problem hiding this comment.
Great TypeScript catch! Explicitly typing Image prevents declaration emit from dropping member JSDoc and @deprecated tags at consumer call sites. Unit tests look solid.
its-Sohan
left a comment
There was a problem hiding this comment.
Code Review: Fix Image export deprecated props visibility
This is an excellent, well-explained PR! The problem is subtle but important for developer experience.
✅ What looks good:
-
Clear problem explanation - The PR description perfectly explains:
- The issue (deprecated tags not showing in editors)
- The root cause (TypeScript declaration emit expanding types from other modules)
- Why the fix works (explicit type annotation preserves JSDoc)
-
Minimal, focused change - The solution is elegant:
- Split into
ImageImpl+ annotated export - Preserves all existing behavior
- Only affects type declaration output
- Split into
-
Comprehensive test - The test is thorough:
- Creates a temporary TypeScript file
- Compiles it and checks JSDoc tags
- Tests all 7 deprecated props
- Includes a control test
-
Non-breaking - The PR correctly notes the types are assignable in both directions
🔧 Minor suggestions:
- Consider adding a comment in the test about the PR number for future reference:
// See https://github.com/vercel/next.js/pull/98597- Test edge case - You might want to test that non-deprecated props don't have the
@deprecatedtag to ensure you're not accidentally adding it to everything
📝 Nits:
-
The test file could be slightly more descriptive:
image-component-deprecated-props.test.tsis fine, butimage-deprecated-props-visibility.test.tsmight be clearer -
The comment in the source code is helpful - good job explaining why the type annotation is needed
💡 Thought:
This is a great example of how TypeScript's declaration emit behavior can affect developer experience. The fix is clean and the explanation is excellent. This should be merged! 🎯
Problem
The
@deprecatedtags onnext/imageprops don't show up in the editor. If you write<Image priority />you get no strikethrough and no hint that you should usepreloadnow.The tags are there in
ImageProps, they just never reach the user.Cause
Imageis declared like this, with the return type inferred:ImagePropslives in another file (shared/lib/get-img-props). When TypeScript expands a type alias from a different module during declaration emit, it drops the JSDoc of all the members. If the alias is in the same file the comments survive, cross file they don't.This is what we ship today:
And this is what we get with the fix:
It recovers the tags for
priority,onLoadingComplete,layout,objectFit,objectPosition,lazyBoundaryandlazyRoot. So every deprecation added to the component since #83351 has been invisible.Is this breaking?
No. The old expanded type and the new one are assignable in both directions, and the props you get in autocomplete are the same (304 entries, same list). Only the way the type is printed changes. Now the deprecated ones are also marked as deprecated in the completion list.
The component is split in two (
ImageImpl+ the annotated export) only to keep the diff small. If I annotate the export directly, Prettier reformats the whole function body and the diff becomes 135 lines for a one line change.Test
test/unit/image-component-deprecated-props.test.tscompiles a small file that importsnext/image, and checks thatgetJsDocTags()returnsdeprecatedfor each of those props. With the old declaration it fails 7 of 8 (the 8th one only checks the props resolve, and passes either way, so it works as a control).I didn't use
expectTypeOflikeinfer-get-server-side-props-type.test.tsdoes, because both types are assignable to each other, so a type equality check passes before and after the fix. It can't catch this.Alternative I tried
Turning
ImagePropsinto aninterfacealso fixes it, and it needs no changes in the component, because interfaces are always referenced by name in the emitted types. I didn't go that way because interfaces don't get an implicit index signature, soconst x: Record<string, unknown> = imagePropsstops compiling.ImagePropsis public, so that would break users. Happy to switch if you prefer it.Also worth noting: with
--isolatedDeclarationsTypeScript already rejects the current code withTS9010: Variable must have an explicit type annotation. So this annotation is what TS wants anyway.