search

NativeScript / android

NativeScript for Android using v8
https://docs.nativescript.org/guide/android-marshalling
Apache License 2.0
518 stars 136 forks source link

feat: URL & URLSearchParams #1801

Closed triniwiz closed 3 months ago

anonrig commented 5 months ago

🚀

shirakaba commented 5 months ago

(Link to the equivalent iOS PR)

This looks way simpler and smaller than the WinterCG PR; I love it. Will be able to review properly later.

@triniwiz Could you explain what precisely this PR does? It appears to add code into test-app, so – being unfamiliar with this repo – I'm unclear whether it would add support for URL into the Android runtime itself, or just makes a proof-of-concept.

Could also you include the licence for Ada, and make clear in the repo the ownership of the code in test-app/README.md? If you can also tell me in the conversation, it will save me from reviewing @anonrig's source which has already been through thorough review.

What would this PR mean for the WinterCG repo, by the way? It seemed like the WinterCG plugins approach would allow people to opt-in to the functionality, and I'm just trying to determine whether this approach would mean we'd have it baked in by default instead.

@anonrig Thanks for engaging with the team! Hope to get a chance to chat with you at some point about cross-foundation collaboration, JS engine/runtime interop and spec conformance 🙂 I'm the new TSC Chair and all of these things excite me, so hope to advance these efforts.

anonrig commented 5 months ago

Can you make sure native script relies on the official Ada rust crate (ref: ada-url available at GitHub.com/ada-url/rust) instead of a fork?

Thanks @shirakaba. Nice to meet with you.

triniwiz commented 5 months ago

This adds a wrapper around the url lib Ada, introducing the URL & URLSearchParams class directly to the runtime.

Doing it this removes the extra step needed to install/setup as it will be available on global.

As for the licence sure I'll need to copy it, I only followed the instructions on the repo 😅 .

The PR in the WCG repo we can remove it 🤷🏽, but yes we would lost the opt-in functionality as it will now be baked in by default .

farfromrefug commented 5 months ago

I personally think we should make that an agnostic plugin. there is no real gain in accessing v8 direclty here, like you dont do 1000 URL use per second or something. we could compare perfs with using JNI but I dont think it would make much of a difference (the only addition is just the JNI layer). We could also set up a plugin repo uing swig to create bindings to JAVA / Objc (or something else if there is better). swig is nice, I need to check if latest versions add less overhead than before. there would be one drawback though. it would not support out of the box future platforms like desktop. but does this ?