Skip to content
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 prefer-blob-reading-methods rule #2065

Merged
merged 7 commits into from Apr 17, 2023

Conversation

NotWoods
Copy link
Contributor

@NotWoods NotWoods commented Apr 7, 2023

Fixes #1269

I opted to allow new Response(blob).arrayBuffer() because it's difficult to flag without type-checking (blob might be a typed array, stream, etc) and because it has better browser support than the blob methods.

@fisker
Copy link
Collaborator

fisker commented Apr 8, 2023

because it's difficult to flag without type-checking (blob might be a typed array, stream, etc)

make sense 👍

@fisker
Copy link
Collaborator

fisker commented Apr 8, 2023

Can you fix the linting problem?

@silverwind
Copy link
Contributor

silverwind commented Apr 10, 2023

It won't trigger on reader.readAsDataURL(), right?

@sindresorhus sindresorhus merged commit 2bb1a04 into sindresorhus:main Apr 17, 2023
22 checks passed
@NotWoods NotWoods deleted the prefer-blob-reading-methods branch April 17, 2023 15:31
@jimmywarting
Copy link

It won't trigger on reader.readAsDataURL(), right?

There are times i think even readAsDataURL is even as equally bad.
Have seen hundreds of ppl on stackoverflow using it along with <img>
it have worse performance than using URL.createObjectURL(blob), it's sync, faster and easier to use. and you don't have to waste time reading the file, convert to/from base64

and most often it's also better to use typed arrays instead of base64 that's ~33% larger in since.

@silverwind
Copy link
Contributor

silverwind commented Apr 17, 2023

I wasn't aware of URL.createObjectURL. If it does what it says, I do agree reader.readAsDataURL should be flagged as well. My use case is a simple convertion of an image Blob to a Data URI.

@NotWoods
Copy link
Contributor Author

This discussion is better for the issues section so it doesn't get lost.

I agree that readAsDataURL has its own issues but it feels like it should be a separate rule. URL.createObjectURL has its own issues - it creates a pointer that should be freed later to avoid memory leaks.

@jimmywarting
Copy link

it creates a pointer that should be freed later to avoid memory leaks.

You get a point there! However... if it's a disc based file. then maybe it don't matter as much. Want to quote this guy on SO:

It mainly depends on what you pass to createObjectURL.

If you pass a File selected by the user from an , then the blobURI you created is a direct pointer to the file on the user's disk, nothing apart this mapping URI-file_path is saved in memory. So in this case, you can create a lot of these without ever revoking it with no real risk.

If you pass a Blob (or a File) that you did generate (or which has been fetched), then the Blob has to be stored in memory, and the blobURI will indeed keep being a pointer to this Blob and its data, protecting it from GC, until the document dies. In this case, don't forget to revoke it when you don't need it anymore.

https://stackoverflow.com/a/49346614/1008999

@jimmywarting
Copy link

On the + side. if you use <img src="blob:url" loading="lazy"> then it could maybe even possible be read lazily on demand from the disc when the image are in viewport.

and it's so much better than reading a hole video as base64 which would be huge.

@silverwind
Copy link
Contributor

silverwind commented Apr 17, 2023

I see URL.createObjectURL does not actually create Data URIs, but /<uuid> style pseudo-URLs that are only valid for current browsing session.

So depending on use case, it's not a replacement for readAsDataURL, like when one needs a portable string. Think UI with a button to copy an image as Data URI to clipboard.

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

Successfully merging this pull request may close these issues.

Rule proposal: prefer-blob-reading-methods
5 participants