Repository navigation
improved PR #85 - #86
farfromrefug wants to merge 29 commits into
Conversation
…/members/properties for name < length
# Conflicts: # dts-generator/gradle/wrapper/gradle-wrapper.properties # dts-generator/src/main/java/com/telerik/dts/DtsApi.java
# Conflicts: # dts-generator/src/main/java/com/telerik/dts/DtsApi.java
# Conflicts: # dts-generator/src/main/java/com/telerik/dts/DtsApi.java
Updates to tackle Kotlin compatibility issues (NativeScript#84)
* handle kotlin parameters without return paramter names with forbidden chars * ensure replaceGenericsInText does not remove > chars far already any types * better detection of open/close namespace to prevent unclosed namespaces * some inner namespaces where not correctly filled/exported
|
@farfromrefug I tested the PR and from my end it's looking good. It seems that you encountered more edge cases than me! Some points from my end:
|
|
Hi @farfromrefug, I was testing again today and something I'd like to highlight is that names are a bit more cryptic now, not sure if it's the intended behaviour or not: 
What you see in red is what was generated with the changes I had pushed, green the changes with this branch. If it's intended behaviour I guess not a problem. Just highlighting it just in case. One more inconsistency, double namespace.. which I'm still checking whether it's a generation issue or not: |
|
@dangrima90 seems like there is a bug there. What lib are you testing with? i need to reproduce |
|
@farfromrefug this is the SDKs I'm testing https://github.com/orgs/Mitek-Systems/packages?repo_name=MiSnap-Android. I think you can download the packages from the link I sent. Else you might need to set up a PAT to download the AAR files: https://github.com/Mitek-Systems/MiSnap-Android?tab=readme-ov-file#how-to-integrate-the-misnap-sdk-using-maven. For example, the screenshot above showing |
|
@dangrima90 i dont have credentials for that file. can you share the aar directly by mail ( dev at akylas dot fr) |
|
@farfromrefug unfortunately I'm being blocked from attaching the aar file as an attachment. Just in case you can download the aar file from here: https://github.com/Mitek-Systems/MiSnap-Android/packages/1577915. Screen.Recording.2025-11-13.at.08.39.50.movDoes this help, or you're still not able to access it? |
|
@dangrima90 i downloaded it thanks. But i dont see broken part of |
|
Hi again @farfromrefug, sorry I might have passed a wrong version. Somehow the latest version is listed as This is the same version I'm testing with Here's an example of the changes (same as before, red = my branch, green = your updates 🙂): For reference I've attached the typings file for the core SDK I had generated so that you can compare: misnap-core.d.ts. Sorry for all the back and forth, if you need me to send more samples let me know, it's not a problem. |
|
@dangrima90 both should be fixed now |
|
@farfromrefug thanks, will try to find time to test today |
|
i added a new flag |
|
Hey @farfromrefug sorry once again for the delay, been busy with other things lately. I will try to review things again and let you know of any anomalies I find. I like the sound of the new flag you added! Will give it a test drive too :) I'll try to run a few tests tomorrow. |
|
Hello @farfromrefug I got the time to test this morning. Things are looking better now thanks. I do still see some very minor issues with naming: 
This is me again comparing what I had before vs the output of this branch. Testing using aar: https://github.com/Mitek-Systems/MiSnap-Android/packages/1577894?version=5.9.0. I don't think the above is a major issue, just maybe keeping the names intact would be helpful. In certain places I notice that whilst before kotlin dependencies were typed as 
This can be observed via: https://github.com/Mitek-Systems/MiSnap-Android/packages/1577896?version=5.9.0. Again I don't know what impact this has, I don't think it affects anything in reality. I leave it to your good judgement whether this requires changes. Other than that I think all is okay. Sorry for listing more items, just sharing what I'm noticing. |
|
@dangrima90 awesome will try to fix remaining issues. but i can already say that the -per-library is pretty awesome. |
|
Sounds good! Happy to help out with testing if you need any help :) |
Typings generated for Kotlin libraries were syntactically broken, and generating them for kotlinx-coroutines (and so for anything that depends on it, such as Mapbox) crashed. This applies the fixes from #86 on top of the Kotlin metadata support that main gained since, at the root cause, and adds a Kotlin fixture and CI so they stay fixed. - Read type parameters with more than one bound; the regex split crashed on kotlinx-coroutines-core. Crashes keep their stack trace instead of being reported as command line mistakes. - Open and close namespaces from the path that is actually open, so a skipped class in a nesting chain no longer leaves `export module {` or unbalanced braces. Classes whose name cannot be spelled (Foo$$serializer, -Base64 facades) are skipped and references to them become any. - Name parameters after the local variable in their JVM slot rather than by position in the LocalVariableTable, which shifted names even in Java libraries and picked up inlined-lambda locals in Kotlin ones. <set-?> takes the property's name and $this$x becomes receiver. - Replace types from ignored namespaces (kotlin.*, java.util.function.*) while converting them instead of with a regex over the output, which left any<...> behind and rewrote string constants. - Declare constants as `static readonly NAME = <literal>`, valid in a declaration file and typed by their value; NaN, the infinities and longs beyond 2^53 keep their type and document the value. - Use the namespace keyword, which TypeScript 6 requires. - Declare a Kotlin companion object's methods as static, since the runtime resolves Outer.Companion to the companion instance. - Leave <clinit> out of an interface's implementation object. - Declare the outer instance an inner class constructor takes, as the runtime requires. - Read an extension function's parameter nullability past its receiver. - Mark the generated file as exempt from eslint. - Test against test/kotlin-fixture, and in CI type-check the expected output with TypeScript 6. Fixes #75, #83, #84 Co-authored-by: Martin Guillon <dev@akylas.fr>
|
Thank you for this, and for tracking down so many of the Kotlin edge cases. These fixes landed in #89 (c10d0bc), rebuilt on top of the Kotlin metadata support main gained in the meantime, since this branch conflicted with it. You're credited as co-author. A few parts were done differently: namespaces are opened from a stack without the brace-balancing pass, parameter names are read by JVM slot instead of being sanitized and truncated, and ignored namespaces are replaced while converting types, which avoids |
This PR add many things which improve typings generation:
-per-librarywhich will generate separated files for libraries. Also the name contains the library name/version which makes it easier to maintain typingsSome commits are listed here while already merged