fix: include file extensions in accept attribute for better cross-browser compatibility#1269
fix: include file extensions in accept attribute for better cross-browser compatibility#1269TheAbMehta wants to merge 1 commit intopingdotgg:mainfrom
Conversation
…wser compatibility Populate the extension arrays in generateClientDropzoneAccept() using the existing @uploadthing/mime-types extension data. This ensures the HTML <input accept> attribute includes both MIME types and their corresponding file extensions (e.g. .jar, .war, .ear), which fixes file picker filtering on Windows/Chrome for types like application/java-archive. Closes #1157
|
|
@TheAbMehta is attempting to deploy a commit to the Ping Labs Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughModified Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
Greptile SummaryThis PR fixes a cross-browser compatibility issue where Windows/Chrome doesn't properly map certain MIME types (like Key changes:
Implementation flow:
The implementation is clean, well-tested, and directly addresses the reported issues. Confidence Score: 5/5
Important Files Changed
Last reviewed commit: 2eb4eac |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/shared/src/component-utils.ts`:
- Around line 61-65: The exts array built from subTypes and extensionsMap may
contain duplicate file extensions; update the exts computation (referencing the
exts variable, subTypes, and extensionsMap) to deduplicate results before
returning/using them — e.g., collect all mapped extensions (the `.${ext}`
values) and remove duplicates (use a Set or Array.filter uniqueness) so the
final exts contains each extension only once.
- Line 10: The import of getExtensions from "@uploadthing/mime-types" pulls in
the entire mimes DB (via the mimes constant and populateMaps) and negates the
documented tree-shaking; either update the comment to state this intentional
tradeoff or refactor to import only the specific category modules needed (e.g.,
import sub-paths like "@uploadthing/mime-types/image" or
"@uploadthing/mime-types/application") and update calls that use
getExtensions/populateMaps accordingly so only required MIME category modules
are bundled.
In `@packages/shared/test/component-utils.test.ts`:
- Around line 46-51: The test is brittle because it asserts exact ordering of
extension arrays returned by generateClientDropzoneAccept; change assertions to
check presence regardless of order by using Jest matchers like
expect(result).toEqual({ "application/java-archive":
expect.arrayContaining([".jar", ".war", ".ear"]) }) or
expect(result).toHaveProperty("application/java-archive",
expect.arrayContaining([...])) in the tests that currently use toEqual
(including the other failing cases around the same tests), so the tests assert
inclusion not order and remain stable against `@uploadthing/mime-types` data
changes.
| * We don't need all the types, and `/application` | ||
| * entrypoint is ~7k gzip which we can shave off | ||
| */ | ||
| import { getExtensions } from "@uploadthing/mime-types"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, explore the repository structure to locate the file
find . -type f -name "component-utils.ts" | head -5Repository: pingdotgg/uploadthing
Length of output: 105
🏁 Script executed:
# List the files in packages/shared/src to understand context
ls -la packages/shared/src/ 2>/dev/null | head -20Repository: pingdotgg/uploadthing
Length of output: 790
🏁 Script executed:
# Read the component-utils.ts file to see the imports and context
cat -n packages/shared/src/component-utils.ts | head -100Repository: pingdotgg/uploadthing
Length of output: 4166
🏁 Script executed:
# Check the `@uploadthing/mime-types` package structure
find node_modules/@uploadthing/mime-types -type f \( -name "*.js" -o -name "*.d.ts" -o -name "*.json" \) 2>/dev/null | grep -E "(index|package)" | head -20Repository: pingdotgg/uploadthing
Length of output: 47
🏁 Script executed:
# Look at the main entry point
cat node_modules/@uploadthing/mime-types/package.json 2>/dev/null | head -30Repository: pingdotgg/uploadthing
Length of output: 47
🏁 Script executed:
# Check the main entry file
cat node_modules/@uploadthing/mime-types/dist/index.d.ts 2>/dev/null | head -50Repository: pingdotgg/uploadthing
Length of output: 47
🏁 Script executed:
# Check the actual implementation of the main entry
cat node_modules/@uploadthing/mime-types/dist/index.js 2>/dev/null | head -100Repository: pingdotgg/uploadthing
Length of output: 47
🏁 Script executed:
# Check if mime-types is part of this monorepo
find . -type d -name "mime-types" 2>/dev/null | grep -v node_modules | head -10Repository: pingdotgg/uploadthing
Length of output: 86
🏁 Script executed:
# Check the root package.json and workspace structure
cat package.json 2>/dev/null | head -50Repository: pingdotgg/uploadthing
Length of output: 1833
🏁 Script executed:
# Look at packages directory structure
ls -la packages/ 2>/dev/nullRepository: pingdotgg/uploadthing
Length of output: 746
🏁 Script executed:
# Search for any references to mime-types in the codebase
rg "mime-types" --type json | head -20Repository: pingdotgg/uploadthing
Length of output: 510
🏁 Script executed:
# Explore the mime-types package structure
ls -la packages/mime-types/Repository: pingdotgg/uploadthing
Length of output: 805
🏁 Script executed:
# Check the source directory
ls -la packages/mime-types/src/ 2>/dev/nullRepository: pingdotgg/uploadthing
Length of output: 595
🏁 Script executed:
# Read the main entry file
cat packages/mime-types/src/index.ts 2>/dev/nullRepository: pingdotgg/uploadthing
Length of output: 3142
🏁 Script executed:
# Check for getExtensions function
rg "getExtensions" packages/mime-types/src/ -A 10Repository: pingdotgg/uploadthing
Length of output: 732
🏁 Script executed:
# Check the size of application.ts
wc -l packages/mime-types/src/application.tsRepository: pingdotgg/uploadthing
Length of output: 108
🏁 Script executed:
# Look at the start of application.ts to understand its scope
head -50 packages/mime-types/src/application.tsRepository: pingdotgg/uploadthing
Length of output: 1124
🏁 Script executed:
# Check the granular entry points to understand their scope
wc -l packages/mime-types/src/{audio,image,text,video}.tsRepository: pingdotgg/uploadthing
Length of output: 231
🏁 Script executed:
# See what happens when getExtensions is called with all categories
grep -n "application" packages/mime-types/src/index.tsRepository: pingdotgg/uploadthing
Length of output: 276
🏁 Script executed:
# Check if there's a way to build extensions from just the granular imports
# Let's see what the structure of each category looks like
head -20 packages/mime-types/src/audio.tsRepository: pingdotgg/uploadthing
Length of output: 456
🏁 Script executed:
# Check if the component-utils.ts actually uses application/* types
grep -n "application" packages/shared/src/component-utils.tsRepository: pingdotgg/uploadthing
Length of output: 170
🏁 Script executed:
# Let's verify if bundler can tree-shake the mimes constant even with the main entry import
# Check the complete mimes object construction in index.ts
sed -n '15,30p' packages/mime-types/src/index.tsRepository: pingdotgg/uploadthing
Length of output: 367
🏁 Script executed:
# Check if there are any other uses of getExtensions in the codebase
rg "getExtensions" --type ts --type tsx -A 2 -B 2Repository: pingdotgg/uploadthing
Length of output: 92
🏁 Script executed:
# Check the actual package.json exports to understand entry points
cat packages/mime-types/package.jsonRepository: pingdotgg/uploadthing
Length of output: 2493
🏁 Script executed:
# Check what's in misc.ts
wc -l packages/mime-types/src/misc.ts && head -20 packages/mime-types/src/misc.tsRepository: pingdotgg/uploadthing
Length of output: 473
🏁 Script executed:
# Verify how this is actually built/bundled - look for build output
find packages/mime-types/dist -type f 2>/dev/null | head -10Repository: pingdotgg/uploadthing
Length of output: 47
🏁 Script executed:
# Check what application types are actually needed by generateClientDropzoneAccept
# Let's see how generateMimeTypes uses the file types
sed -n '29,51p' packages/shared/src/component-utils.tsRepository: pingdotgg/uploadthing
Length of output: 1037
getExtensions from the main entry pulls the full MIME database into the bundle, contradicting the documented tree-shaking strategy.
The file's comment (lines 5–9) explicitly describes using granular sub-path imports (/audio, /image, etc.) specifically to avoid the ~7 kB gzip cost of the full database. However, the import on line 10 (import { getExtensions } from "@uploadthing/mime-types") pulls from the main entry point, which creates a single mimes constant that combines all categories—including application.ts (2,653 lines), video.ts, text.ts, audio.ts, image.ts, and misc.ts.
The getExtensions() function calls populateMaps(), which iterates through the entire mimes object. Because the bundler cannot distinguish which MIME types will be looked up at runtime, it must include the full object statically—the lazy initialization only defers the CPU cost of building the map at runtime, not the bundle inclusion cost. The complete database is compiled into the bundle regardless.
This is a real regression: application.ts alone is 2,653 lines. If application MIME types (like application/pdf for the "pdf" type) are necessary, consider either:
- Updating the comment to reflect the intentional tradeoff, or
- Refactoring to import only the specific category modules needed at the point of use.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/shared/src/component-utils.ts` at line 10, The import of
getExtensions from "@uploadthing/mime-types" pulls in the entire mimes DB (via
the mimes constant and populateMaps) and negates the documented tree-shaking;
either update the comment to state this intentional tradeoff or refactor to
import only the specific category modules needed (e.g., import sub-paths like
"@uploadthing/mime-types/image" or "@uploadthing/mime-types/application") and
update calls that use getExtensions/populateMaps accordingly so only required
MIME category modules are bundled.
| const exts = subTypes.flatMap((mime) => { | ||
| const mimeExts = | ||
| extensionsMap[mime as keyof typeof extensionsMap] ?? []; | ||
| return mimeExts.map((ext) => `.${ext}`); | ||
| }); |
There was a problem hiding this comment.
Possible duplicate extensions for generic types — consider deduplicating.
When processing a generic type like "image", generateMimeTypes returns a comma-joined string containing many specific subtypes (image/png, image/jpeg, image/jpg, …). Two different MIME types can legitimately share the same extension (e.g., both image/jpeg and image/pjpeg map to .jpg/.jpeg). The current flatMap accumulates all extensions without deduplication, so the resulting array can contain duplicates. While browsers tolerate this in the accept attribute, it's unclean.
♻️ Suggested fix
- return [type, exts];
+ return [type, [...new Set(exts)]];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/shared/src/component-utils.ts` around lines 61 - 65, The exts array
built from subTypes and extensionsMap may contain duplicate file extensions;
update the exts computation (referencing the exts variable, subTypes, and
extensionsMap) to deduplicate results before returning/using them — e.g.,
collect all mapped extensions (the `.${ext}` values) and remove duplicates (use
a Set or Array.filter uniqueness) so the final exts contains each extension only
once.
| it("includes file extensions for specific MIME types", () => { | ||
| const result = generateClientDropzoneAccept(["application/java-archive"]); | ||
| expect(result).toEqual({ | ||
| "application/java-archive": [".jar", ".war", ".ear"], | ||
| }); | ||
| }); |
There was a problem hiding this comment.
toEqual with ordered extension arrays is brittle against MIME-database updates.
The tests at lines 46–51, 55–57, and 89–94 assert the exact ordered content of extension arrays (e.g., [".jar", ".war", ".ear"]). The order is determined by @uploadthing/mime-types's internal data, which could change on any package bump (new extension added, order changed). This would silently break these tests without any functional regression.
Prefer expect.arrayContaining(...) so the tests verify presence, not order:
♻️ Proposed fix
- expect(result).toEqual({
- "application/java-archive": [".jar", ".war", ".ear"],
- });
+ expect(result["application/java-archive"]).toEqual(
+ expect.arrayContaining([".jar", ".war", ".ear"]),
+ );
+ expect(result["application/java-archive"]).toHaveLength(3);- expect(result).toEqual({
- "application/pdf": [".pdf"],
- });
+ expect(result["application/pdf"]).toEqual(
+ expect.arrayContaining([".pdf"]),
+ );- expect(result["application/java-archive"]).toEqual([
- ".jar",
- ".war",
- ".ear",
- ]);
- expect(result["application/pdf"]).toEqual([".pdf"]);
+ expect(result["application/java-archive"]).toEqual(
+ expect.arrayContaining([".jar", ".war", ".ear"]),
+ );
+ expect(result["application/pdf"]).toEqual(
+ expect.arrayContaining([".pdf"]),
+ );Also applies to: 84-95
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/shared/test/component-utils.test.ts` around lines 46 - 51, The test
is brittle because it asserts exact ordering of extension arrays returned by
generateClientDropzoneAccept; change assertions to check presence regardless of
order by using Jest matchers like expect(result).toEqual({
"application/java-archive": expect.arrayContaining([".jar", ".war", ".ear"]) })
or expect(result).toHaveProperty("application/java-archive",
expect.arrayContaining([...])) in the tests that currently use toEqual
(including the other failing cases around the same tests), so the tests assert
inclusion not order and remain stable against `@uploadthing/mime-types` data
changes.
1 participant
Summary
Fixes #1157. Also addresses #949 and #267.
Problem: On Windows/Chrome, certain MIME types like
application/java-archiveare not properly mapped to file extensions in the native file picker. When the HTML<input accept="application/java-archive">attribute contains only the MIME type, the file picker shows "All Files (.)" instead of filtering to.jarfiles.Fix:
generateClientDropzoneAccept()now populates the extension arrays using the existing@uploadthing/mime-typesextension data via the lazygetExtensions()getter. For example:When
acceptPropAsAcceptAttr()flattens theAcceptProp, it already handles extensions (theisExt()check), so the resultingacceptattribute becomes"application/java-archive,.jar,.war,.ear"— which Windows/Chrome can properly filter.Changes
packages/shared/src/component-utils.ts: ImportgetExtensionsfrom@uploadthing/mime-typesand use it ingenerateClientDropzoneAccept()to look up file extensions for each MIME type. Handles comma-joined strings from generic types (e.g."image") by splitting and collecting extensions for each sub-type.packages/shared/test/component-utils.test.ts: Added 6 tests forgenerateClientDropzoneAcceptcovering specific MIME types, thepdfshorthand,blobtype, generic types likeimage, unknown MIME types, and multiple file types.Notes
getExtensions()getter rather than importing the fullapplicationmodule directly, keeping bundle impact minimaldropzone-utils.ts— the existingacceptPropAsAcceptAttr()already handles extensions correctlyDisclosure
This PR was authored with the assistance of Claude (LLM) to help understand the codebase and structure the implementation and description.