Skip to content

chore: add missing mime types #56

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 18, 2024

Conversation

r5dy1n
Copy link

@r5dy1n r5dy1n commented Jun 12, 2022

What kind of change does this PR introduce?

  • Bug Fix
  • Feature
  • Refactoring
  • Style
  • Build
  • Chore
  • Documentation
  • CI

Did you add tests for your changes?

  • Yes, my code is well tested
  • Not relevant

If relevant, did you update the documentation?

  • Yes, I've updated the documentation
  • Not relevant

Summary
Explain the motivation for making this change. What existing problem does the pull request solve?
Try to link to an open issue for more information.

  • Add missing MIME types to support more files. File type is undefined when dropping a file that isn't in the COMMON_MIME_TYPES using React Dropzone

Does this PR introduce a breaking change?
If this PR introduces a breaking change, please describe the impact and a migration path for existing applications.
['ico', 'image/vnd.microsoft.icon'] is now '[ico' => 'image/x-icon']

Other information

@rolandjitsu rolandjitsu merged commit 24faa1b into react-dropzone:master Feb 18, 2024
Copy link

github-actions bot commented Oct 4, 2024

🎉 This PR is included in version 0.7.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@mxdvl
Copy link

mxdvl commented Jan 17, 2025

Has there been any discussion of the impact of this change on the size of the bundle? From my understanding this has increased the file size of this package five folds, going from 2.6KB to a whooping 12KB. From my understanding many of the MIME types are very uncommon. It’s pretty telling from the minified output of 2.1.2 that 80% of this package is MIME type and extension matching.

One possible option would be to export file types as a convenience to enable consumers to optionally select a subset that’s relevant to them? For example:

export const COMMON_FILE_TYPES = new Map([
  // top 5 or 20 file types by usage…
]);

export const ALL_FILE_TYPES = new Map([
 ...COMMON_FILE_TYPES,
 // provided as an export for convenience
])

// here an optional second argument default for convenience an preventing breaking changes
export async function fromEvent(evt: Event | any, fileTypes = COMMON_FILE_TYPES): Promise<(FileWithPath | DataTransferItem)[]> {
}

@rolandjitsu
Copy link
Collaborator

Has there been any discussion of the impact of this change on the size of the bundle? From my understanding this has increased the file size of this package five folds, going from 2.6KB to a whooping 12KB. From my understanding many of the MIME types are very uncommon. It’s pretty telling from the minified output of 2.1.2 that 80% of this package is MIME type and extension matching.

One possible option would be to export file types as a convenience to enable consumers to optionally select a subset that’s relevant to them? For example:

export const COMMON_FILE_TYPES = new Map([
  // top 5 or 20 file types by usage…
]);

export const ALL_FILE_TYPES = new Map([
 ...COMMON_FILE_TYPES,
 // provided as an export for convenience
])

// here an optional second argument default for convenience an preventing breaking changes
export async function fromEvent(evt: Event | any, fileTypes = COMMON_FILE_TYPES): Promise<(FileWithPath | DataTransferItem)[]> {
}

Thanks for bringing this up. What we could probably do is provide an option to users to not get the common mime types bundled (not sure if tree shaking can help as you suggested or maybe we can separate the repos and provide the common mime types as a separate pkg).

I guess we'll track that in #127.

@mxdvl
Copy link

mxdvl commented Feb 23, 2025

not sure if tree shaking can help

Since writing the above I discovered no bundler will tree-shake away default parameters, so my example above would NOT work as-is 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants