NativeScript / android

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

Update getAppResourcesPath() to use the new configs #1634

Closed rigor789 closed 4 years ago

rigor789 commented 4 years ago

@NathanaelA we need to update these for 7.0 to read from the new config file. I think we should probably create a new CLI command that the gradle script can invoke and read the output - because the configs are no longer simple json files. As a quick fix, we could read the value with some regex (though that'd be easy to break in the configs - since it's js/ts).

The current implementation could be kept as a fallback.

https://github.com/NativeScript/android-runtime/blob/b10b6d625807f0d65dfebe2649cc37cfb124dd6e/test-app/gradle-helpers/paths.gradle#L19-L37

https://github.com/NativeScript/android-runtime/blob/f71130432d1fed102776bb8372906aadda82d3fe/test-app/app/build.gradle#L137-L157

NathanaelA commented 4 years ago

@rigor789 - How about the CLI create the nsconfig.json file if the nsconfig.js file version of it exists; then no other tooling needs to change at this point, and it is then fully backwards compatible with older runtimes. Frequently we have people try using older runtimes to see when a issue has occurred; so breaking the CLI's compatibility should actually be fixed first...

rigor789 commented 4 years ago

The goal and we are 99% there is to get rid of redundant configs, package.jsons, special keys etc.

What I think we can/should do to keep older runtimes working, while not forcing an nsconfig.json on newer runtimes is to only generate the nsconfig.json when the runtime version is less than 7.0.0 and for 7 and above, we can add the new logic for reading the config value through the CLI.