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

File, FileReader, Blob, ArrayBuffer are replacing Javascript native code instead of extending it. #572

Open
LucasBourgeois opened this issue Jun 26, 2023 · 5 comments

Comments

@LucasBourgeois
Copy link

Bug Report

Problem

Many of this plugin classes are overriding Javascript native classes instead of implements them properly

What is expected to happen?

Plugin Blob, File, ArrayBuffer, FileReader classes should not replace Javascript native code and extend native JS API's

What does actually happen?

For exemple, if i do a const file = new File([], 'yoyo'), I can't use Blob() functions like file.arrayBuffer() as File isn't extending Blob interface.
Also, file.name is an array containing a Blob item.

It cause unexpected behavior between browser web native Javascript File implementation and phone device cordova one.
In addition, we must add trash code to handle this.

Information

https://developer.mozilla.org/en-US/docs/Web/API/Blob
https://developer.mozilla.org/en-US/docs/Web/API/File
https://developer.mozilla.org/fr/docs/Web/API/FileReader

Command or Code

Just init a new File() or an new ArrayBuffer() and compare them with browser result, they are not as Javascript define them.

Environment, Platform, Device

Cordova-android 11, Android 13 device, Chrome.

Version information

    "cordova-plugin-file": "^6.0.2",

Checklist

  • [ X] I searched for existing GitHub issues
  • [X ] I updated all Cordova tooling to most recent version
  • [ X] I included all the necessary information above
@LucasBourgeois
Copy link
Author

any news @breautek

@LucasBourgeois
Copy link
Author

LucasBourgeois commented Dec 9, 2024

Saw this : still really evil to not follow the MDN
#316 (comment)

@breautek
Copy link
Contributor

breautek commented Dec 9, 2024

Saw this : still really evil to not follow the MDN #316 (comment)

I haven't really had a chance to take a look, but changing anything is probably going to be a hefty project unfortunately. It's not something that I can prioritise development time either. If someone wants to try to tackle this issue, then the Dev Mailing List is a place to discuss solutions how move forward, if that solution is a big change.

The plugin shouldn't be affecting Blob or ArrayBuffer symbols, I don't see anywheres where those symbols are overwritten. It does overwrite File and FileReader (and a few others, implementing the old and now defunct filesystem spec).

It likely did replacements because back in the day there was probably nothing to extend as most of these symbols just simply didn't exist in webview environments. Though today that probably has changed, so extending the native browser objects and overriding bits needed to communicate over the native bridge might be feasible. This would need a PoC to test with.

I did do a quick review this morning and File object is going to be problematic, as it doesn't seem to follow the standard at all, at least for it's constructor. The constructor params probably should change to match the standard (and then it can probably extend the native browser File which should satisfy Type checks and also inherit the blob features as well). Though user code shouldn't be constructing File objects themselves, I think File is still part of the public API and thus changing it would be considered a breaking change. I do agree not following the standard that it overwrites is evil, as far as I can tell, it always broke the specification on for this.

@LucasBourgeois
Copy link
Author

understood, i did write the mail.

Well, it's a breaking change for the good !
Im no Java or Swift developper, this being said :

if keeping native implementation like it's today, if we stop overriding File and arrayBuffer but exposing your implementation and make necessary changes on js side to transform from the JS standards API to yours should be ok.

Because of dependency versionning, it's not really a issue to create a breaking change. If others libs arent maintained, it's not an issue, if they are, they can keep the legacy implementation until they do the changes.

having a legacy and a fixed dist may also be a solution ?

regards

@erisu
Copy link
Member

erisu commented Dec 10, 2024

Hi @LucasBourgeois ,

I noticed that your email was caught by the moderation tool, likely because you might not be registered on the mailing list.

I’ve approved your email, so it should now be delivered to the thread.

To ensure that future responses are delivered directly to you, you may want to register for the mailing list.

You can find the subscription address for the dev mailing list on Cordova's website. The link to the "Mailing List" page is located at the bottom of the site.

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

No branches or pull requests

3 participants