Skip to content

Conversation

@majiayu000
Copy link

Summary

  • Makes the attachments property optional in FileUploadModalData interface
  • Aligns TypeScript types with runtime behavior where attachments may be undefined
  • Adds type test to verify the optional property behavior

Issue

Fixes #11359

Test Plan

  • Added type test in typings/index.test-d.ts to verify attachments can be undefined
  • All existing tests pass (pnpm run test --filter=discord.js)
  • Type definitions are valid (tsd passes)

Changes

  • packages/discord.js/typings/index.d.ts: Changed attachments from required to optional in FileUploadModalData
  • packages/discord.js/typings/index.test-d.ts: Added type assertion test for optional attachments

The `attachments` property in `FileUploadModalData` can be undefined
when the resolved data doesn't include attachments, but the TypeScript
typings previously marked it as required.

This change:
- Makes `attachments` optional in `FileUploadModalData` interface
- Adds a type test to verify the fix

This aligns with the JSDoc documentation in ModalSubmitInteraction.js
which already marks `attachments` as optional (`[attachments]`).

Fixes discordjs#11359
@vercel
Copy link

vercel bot commented Dec 14, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Review Updated (UTC)
discord-js Skipped Skipped Dec 14, 2025 7:13am
discord-js-guide Skipped Skipped Dec 14, 2025 7:13am

@vercel vercel bot temporarily deployed to Preview – discord-js-guide December 14, 2025 07:13 Inactive
@vercel vercel bot temporarily deployed to Preview – discord-js December 14, 2025 07:13 Inactive
@coderabbitai
Copy link

coderabbitai bot commented Dec 14, 2025

📝 Walkthrough

Walkthrough

The attachments field in the FileUploadModalData interface is made optional to align TypeScript typings with runtime behavior, where the field can be undefined. A corresponding test verifies the optional nature of this property.

Changes

Cohort / File(s) Summary
Type definitions
packages/discord.js/typings/index.d.ts
Made attachments property optional in FileUploadModalData interface by adding ? modifier
Type tests
packages/discord.js/typings/index.test-d.ts
Added import of FileUploadModalData and test assertion confirming attachments property accepts undefined

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Single, straightforward type modifier change (? added to one field)
  • Minimal test addition that directly validates the type change
  • No complex logic or multi-faceted reasoning required

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: making FileUploadModalData.attachments optional.
Description check ✅ Passed The description is well-related to the changeset, providing summary, issue reference, test plan, and specific file changes.
Linked Issues check ✅ Passed The PR successfully addresses issue #11359 by making attachments optional in FileUploadModalData, aligning types with runtime behavior where attachments can be undefined.
Out of Scope Changes check ✅ Passed All changes are directly related to the linked issue: making the attachments property optional and adding tests to verify the behavior.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c4fc79a and 8ef07fe.

📒 Files selected for processing (2)
  • packages/discord.js/typings/index.d.ts (1 hunks)
  • packages/discord.js/typings/index.test-d.ts (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/discord.js/typings/index.test-d.ts (2)
packages/discord.js/typings/index.d.ts (3)
  • FileUploadModalData (2564-2568)
  • ReadonlyCollection (1083-1083)
  • Attachment (2268-2287)
packages/collection/src/collection.ts (1)
  • ReadonlyCollection (6-10)
packages/discord.js/typings/index.d.ts (1)
packages/collection/src/collection.ts (1)
  • ReadonlyCollection (6-10)
🔇 Additional comments (3)
packages/discord.js/typings/index.d.ts (1)

2564-2568: Good type fix: FileUploadModalData.attachments optional matches runtime + existing getUploadedFiles() contract.
No issues with this change; it aligns with BaseInteractionResolvedData.attachments? already being optional and nudges consumers toward null-safe access.

packages/discord.js/typings/index.test-d.ts (2)

103-103: LGTM! Import correctly added.

The FileUploadModalData import is properly placed in alphabetical order and is necessary for the type test added later in the file.


3049-3051: LGTM! Test correctly validates the fix for issue #11359.

The type test properly verifies that FileUploadModalData.attachments is optional and can be undefined, which aligns with the runtime behavior and the interface change in index.d.ts.


Comment @coderabbitai help to get the list of available commands and usage tips.

@vakiliner
Copy link
Contributor

Duplicate of #11363

@almeidx almeidx closed this Dec 17, 2025
@github-project-automation github-project-automation bot moved this from Todo to Denied in discord.js Dec 17, 2025
@majiayu000 majiayu000 deleted the fix/issue-11359-fileupload-attachments-optional branch December 18, 2025 03:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Denied

Development

Successfully merging this pull request may close these issues.

ModalSubmitInteraction field component.attachments may be undefined despite typings

3 participants