-
Notifications
You must be signed in to change notification settings - Fork 887
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
Add custom webpack loader to remove unused mimetypes from mime-db #5148
Conversation
Co-authored-by: Jason <84899178+jasonhenriquez@users.noreply.github.com>
if (mimeType.startsWith('image/') && original[mimeType].extensions && | ||
(!mimeType.startsWith('image/x-') || mimeType === 'image/x-icon' || mimeType === 'image/x-ms-bmp') && | ||
(!mimeType.startsWith('image/vnd.') || mimeType === 'image/vnd.microsoft.icon')) { | ||
|
||
// Only the extensions field is needed, see: https://github.com/kevva/ext-list/blob/v2.2.2/index.js | ||
reduced[mimeType] = { | ||
extensions: original[mimeType].extensions | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just make it easier to read
Coz the logic seems to be
- Add all
image/
MIME types not matching specific prefixes with extensions - Add specific MIME types
if (mimeType.startsWith('image/') && original[mimeType].extensions && | |
(!mimeType.startsWith('image/x-') || mimeType === 'image/x-icon' || mimeType === 'image/x-ms-bmp') && | |
(!mimeType.startsWith('image/vnd.') || mimeType === 'image/vnd.microsoft.icon')) { | |
// Only the extensions field is needed, see: https://github.com/kevva/ext-list/blob/v2.2.2/index.js | |
reduced[mimeType] = { | |
extensions: original[mimeType].extensions | |
} | |
} | |
if (!mimeType.startsWith('image/')) { continue } | |
if (original[mimeType].extensions == null) { continue } | |
let shouldBeAdded = false | |
if (!mimeType.startsWith('image/x-') && !mimeType.startsWith('image/vnd.')) { | |
shouldBeAdded = true | |
} else { | |
shouldBeAdded = mimeType === 'image/x-icon' || | |
mimeType === 'image/x-ms-bmp' || | |
mimeType === 'image/vnd.microsoft.icon' | |
} | |
if (!shouldBeAdded) { continue } | |
// Only the extensions field is needed, see: https://github.com/kevva/ext-list/blob/v2.2.2/index.js | |
reduced[mimeType] = { | |
extensions: original[mimeType].extensions | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Current code is easier to read imo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @efb4f5ff-1298-471a-8973-3d47447115dc I think the current code is more readable, also your code changes the logic, resulting in a different output.
I prefer the or/||
operator over writing lots of single line ifs that have the same outcome, as to me it's a lot more readable, especially when the variable names are short.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @efb4f5ff-1298-471a-8973-3d47447115dc I think the current code is more readable, also your code changes the logic, resulting in a different output.
I prefer the or/||
operator over writing lots of single line ifs that have the same outcome, as to me it's a lot more readable, especially when the variable names are short.
Also we really don't need to check for null
here, as this code runs on a json file with a known structure so the only options are non-existent/undefined
or an array, so checking for loose equality with null
is rather confusing. Also the truthy check already covers both undefined
and null
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file won't be changed frequently so it's fine
Even if it were going to get frequent changes, using a more verbose, less readable code style, that is also less efficient, would still be a bad idea in my book. |
* development: Update playlist name with title (FreeTubeApp#5150) User playlists as grid (FreeTubeApp#4949) Add custom webpack loader to remove unused mimetypes from mime-db (FreeTubeApp#5148) ^ Update GH action eps1lon/actions-label-merge-conflict (FreeTubeApp#5034) Translated using Weblate (Italian) Translated using Weblate (Serbian) Translated using Weblate (Estonian) Translated using Weblate (Bulgarian) Translated using Weblate (Spanish) Translated using Weblate (Italian) Translated using Weblate (Polish) Translated using Weblate (Portuguese (Brazil)) Translated using Weblate (French) Translated using Weblate (Chinese (Traditional)) Translated using Weblate (Chinese (Simplified)) Fix gap next to banner when Hide Side Bar Labels is enabled (FreeTubeApp#5120)
Add custom webpack loader to remove unused mimetypes from mime-db
Pull Request Type
Description
To be able to offer it's
Save xyz As...
context menu entrieselectron-context-menu
needs to know what file extension it needs to give to the downloaded files, which is a bit difficult as web uses mime types/media types instead of file extensions. That's wheremime-db
comes in, it's a gigantic list of mime types and their file extensions where they are known.As mentioned in the previous paragragh that list is gigantic (see it for yourself: https://github.com/jshttp/mime-db/blob/master/db.json) and takes up more than half of the
main.js
file's file size. So I set about to rectify that (well I've known about it for a while now, but I've only just come up with a good solution for it). As we only use theSave Image As...
context menu entry, we only need the image ones, which already gets rid of a large number of them, we can also get rid of some non-standard ones.The new webpack loader only takes a few milliseconds to run on my machine, to take the
mime-db
module from143 KiB
to1.98 KiB
, so low impact on the build time but high impact on the file size.All that means that the
main.js
file went from being247 KiB
to106 KiB
!!!Screenshots
Extracts from the webpack build log for the
main.js
file.Before:
After:
Testing
yarn run pack
dist/main.js
Desktop