Repository navigation
PRD-478 Quickstart update - #676
Conversation
- Restore compatibility with recent React Native and Android Gradle Plugin versions - Prevent the initial blank MapView request from causing an error on iOS
| @@ -0,0 +1,34 @@ | |||
| // Expo config plugin that adds the Situm Maven repository to Android projects. | |||
There was a problem hiding this comment.
Esto es un config plugin de Expo
https://docs.expo.dev/config-plugins/introduction/
Para añadir un repositorio al build.gradle mediante un plugin de Expo, la documentación ofrece una vía oficial mediante una propiedad llamada extraMavenRepos https://docs.expo.dev/versions/latest/sdk/build-properties/
Para utilizar esta vía oficial, el integrador debe instalar el paquete expo-build-properties (este es oficial y está documentado en el enlace) y el contenido de este archivo debería ser algo como:
{
"expo": {
"plugins": [
[
"expo-build-properties",
{
"android": {
"extraMavenRepos": [
"https://repo.situm.com/artifactory/libs-release-local"
]
}
}
]
]
}
}
No nos interesa que el integrador tenga que instalar una dependencia a mayores, así que este config plugin llama directamente a la api interna de Expo para añadir el repositorio de Situm. Debería funcionar exactamente igual que si lo añades de la manera oficial, pero sin necesitar instalar expo-build-properties.
| "expo": { | ||
| "optional": true | ||
| } | ||
| }, |
There was a problem hiding this comment.
Hace que si expo está instalado en el proyecto, el plugin utilice esa instalación. Ayuda a que se mantengan las mismas estructuras de datos si cambian entre versiones y a que el plugin de Expo reconozca el paquete correctamente.
No fuerza la instalación de Expo ni lanza ninguna advertencia si el integrador no lo tiene instalado.
| const _onShouldStartLoadWithRequest = (request: any) => { | ||
| if (request?.url === "about:blank") { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
En iOS lanzaba errores al iniciar el mapview porque intentaba abrir la página "about:blank" con el Linking.openURL. En la app de ejemplo esto no ocurría porque tiene definido un onExternalLinkClicked y se llama a esa función en su lugar.
There was a problem hiding this comment.
En el plugin de Cordova no se interceptan los enlaces abiertos, así que es Capacitor el que lo gestiona y evita el error que ocurre en React Native.
En Flutter no se lanzan errores porque la lógica de interceptar las urls solo se aplica una vez el mapview ha terminado de cargar, y los about:blank vienen antes. Además se llamaría a esta función para abrir enlaces en la parte nativa de iOS. En ella se comprueba si se puede abrir el enlace antes de intentarlo, por lo que tampoco lanzaría error aunque se intentase.
Al final hemos decidido Cristian y yo no complicarlo más y dejarlo con este comportamiento que es el mismo que en el sdk de iOS: si es un about:blank no abrirlo ni en el viewer ni en el navegador.
Los about:blank los genera el Sentry Replay y este tiene un fallback para cuando no puede abrirlos, así que debería seguir funcionando bien.
|
|
||
| release { | ||
| minifyEnabled false | ||
| proguardFiles getDefaultProguardFile("proguard-android.txt"), "proguard-rules.pro" |
There was a problem hiding this comment.
Era una configuración que venía por defecto en React Native antiguo. Lo he quitado porque los proyectos nuevos de React Native vienen con AGP 9 y este ya no soporta la configuración de proguard-android.txt.
Este fue el error:
A problem occurred evaluating project ':situm_react-native'.
> `getDefaultProguardFile('proguard-android.txt')` is no longer supported since it includes `-dontoptimize`, which prevents R8 from performing many optimizations. Instead use `getDefaultProguardFile('proguard-android-optimize.txt)`, and if needed, temporarily use `-dontoptimize` in a custom keep rule file while fixing breakages
La configuración por defecto de React Native ya trae el minifyEnabled en false, lo cual desactiva el proguard. Además de que al ser un plugin, esta setting nunca se aplica, la que se aplica es la de la app integradora. Debería ser seguro quitarlo.
|
|
||
| const SITUM_MAVEN_REPOSITORY = | ||
| "https://repo.situm.com/artifactory/libs-release-local"; | ||
| const { createBuildGradlePropsConfigPlugin } = AndroidConfig.BuildProperties; |
There was a problem hiding this comment.
Comentas que é unha API interna. Que ocorre neste código si cambian o nome de createBuildGradlePropsConfigPlugin? O código sería o suficientemente defensivo como para tolerar o problema, ou cascaría?
There was a problem hiding this comment.
Se podría liar bastante, si llama a una función que no existe Expo se detiene y no genera las carpetas de Android e iOS.
| @@ -0,0 +1,16 @@ | |||
| // Expo config plugin that adds the Situm Maven repository to Android projects. | |||
| const { AndroidConfig } = require("expo/config-plugins"); | |||
There was a problem hiding this comment.
Analizaches withGradleProperties?
Quizais poida ser unha alternativa máis documentada?
O que xa falamos, por min podemos darlle para adiante a este apaño automático, pero acaba de saltarme esto e igual merece a pena botarlle un ollo.
Tamén me preocupa que sexa resistente a erros :)
There was a problem hiding this comment.
Me parece correcto, si bien el código resulta más complejo, es una API pública que es menos probable que incurra en errores críticos.
| return config; | ||
| }); | ||
|
|
||
| module.exports = withSitumMavenRepository; |
There was a problem hiding this comment.
He añadido comentarios explicando qué hace cada parte del código para que, si hay que revisar el plugin en un tiempo, no se nos olvide la lógica que se aplicaba. Me pareció lo correcto al ser una parte más aislada del resto del plugin y ser el único sitio que utiliza interfaces de Expo.
R-lagoman
left a comment
R-lagoman
left a comment
There was a problem hiding this comment.
Por min queda aprobada. Algún detalle que comentas escápaseme, tería que invertir
No description provided.