Migrate to "Expo modules", from react-native-unimodules
#5133
Labels
dependencies
Pull requests that update a dependency file
Comments
gnprice
added a commit
to gnprice/zulip-mobile
that referenced
this issue
Dec 30, 2021
This isn't quite the latest version; for now we'll use v9.x in order to stick with "unimodules". Issue zulip#5133 is open for the migration that'll let us advance to v10.x. Include a libdef, generated with flowgen with small manual fixups as described in comments. The libdef is actually from a newer version because that's what I tried first; I tried rerunning flowgen on the older version, and the differences in the new one are pure compatible improvements (cleaning up some type definitions; adding comments; replacing `any[]` as the type for `args` on `executeSql`.)
chrisbobbe
pushed a commit
to gnprice/zulip-mobile
that referenced
this issue
Jan 5, 2022
This isn't quite the latest version; for now we'll use v9.x in order to stick with "unimodules". Issue zulip#5133 is open for the migration that'll let us advance to v10.x. Include a libdef, generated with flowgen with small manual fixups as described in comments. The libdef is actually from a newer version because that's what I tried first; I tried rerunning flowgen on the older version, and the differences in the new one are pure compatible improvements (cleaning up some type definitions; adding comments; replacing `any[]` as the type for `args` on `executeSql`.)
gnprice
added a commit
that referenced
this issue
Jan 11, 2022
This isn't quite the latest version; for now we'll use v9.x in order to stick with "unimodules". Issue #5133 is open for the migration that'll let us advance to v10.x. Include a libdef, generated with flowgen with small manual fixups as described in comments. The libdef is actually from a newer version because that's what I tried first; I tried rerunning flowgen on the older version, and the differences in the new one are pure compatible improvements (cleaning up some type definitions; adding comments; replacing `any[]` as the type for `args` on `executeSql`.)
sumj25
pushed a commit
to sumj25/zulip-mobile
that referenced
this issue
Jan 12, 2022
This isn't quite the latest version; for now we'll use v9.x in order to stick with "unimodules". Issue zulip#5133 is open for the migration that'll let us advance to v10.x. Include a libdef, generated with flowgen with small manual fixups as described in comments. The libdef is actually from a newer version because that's what I tried first; I tried rerunning flowgen on the older version, and the differences in the new one are pure compatible improvements (cleaning up some type definitions; adding comments; replacing `any[]` as the type for `args` on `executeSql`.)
|
OK, I've poked around a bit, trying to understand the changes listed here with instructions to "apply" them to our project. Nothing there really helps us understand why these are the right changes to make, or exactly what'll happen if we make them. My pretty good guess right now is that those diffs correspond to a subset of the changes in a "template app" maintained by Expo: the I wanted to know what commits led to those particular changes, from earliest to latest; here they are (it's not a huge list): I think we can do basically what we've been doing for the last few React Native upgrades: study each of these commits to gather tasks we want to do on our own project (ignoring changes that are badly reasoned, reverted later, or just not relevant to our project), then do those tasks in our own disciplined way with well-ordered, clear, coherent commits. I said above that the changes dumped into the migration guide are a subset of the diff I indicated…maybe we'll find out if some changes are intentionally omitted, and if we want to ignore them? The following files are touched in the diff but don't get mentioned in the migration guide:
Our And the following files are mentioned, but in broad strokes that don't capture everything that appears in the diff:
So we'll want to look into those more closely. |
This comment has been minimized.
This comment has been minimized.
chrisbobbe
added a commit
to chrisbobbe/zulip-mobile
that referenced
this issue
Jan 25, 2022
Specifically, most of the changes to templates/expo-template-bare-minimum in expo/expo@05f2ed0c7, and relevant fixups from expo/expo@d3f983699, expo/expo@9781212eb, and expo/expo/87a3d02aa. We also include this flavor of change as it appears in expo/expo@9781212eb. Might as well; shrug. See expo/expo#13901 (comment) . expo/expo@05f2ed0c7 sloppily added a new import in the Podfile, even though the commit was advertised as a pure refactor: require File.join( `node --print "require.resolve('@unimodules/react-native-adapter/package.json')"`, "../scripts/autolinking" ) We won't ever need that import, so we leave it out. It looks like the relevant script was eventually moved to the `expo` package. We'll import the script from there when we switch from "Unimodules" to "Expo modules" (zulip#5133), which is the first time we'll need it.
chrisbobbe
added a commit
to chrisbobbe/zulip-mobile
that referenced
this issue
Jan 25, 2022
Another change in the direction of the previous commit. Could take it or leave it; shrug. See the code comment for where we got this coherent lump of goo. I don't like it, especially since this doesn't solve any problem we're likely to encounter. Could just skip it? Anyway, I collected this while gleaning Expo's commits for changes we *are* interested in following, for zulip#5133. It would be lovely if Expo would propose these kinds of fixes in `react-native` itself. I think they realize that that'd be best, but just haven't had time; see expo/expo#13901 (comment) : > That's a good point that we should make that change in > `react-native` as well, thanks for pointing that out 🙇
chrisbobbe
added a commit
to chrisbobbe/zulip-mobile
that referenced
this issue
Jan 25, 2022
Like we did in 196a316. Changelog: https://github.com/JetBrains/kotlin/blob/master/ChangeLog.md#1610 This seems to fix the following error we'd see on running `tools/test native --all-files` after switching from Unimodules to Expo modules (zulip#5133): ``` e: /Users/chrisbobbe/dev/zulip-mobile/node_modules/expo-sqlite/android/src/main/java/expo/modules/sqlite/SQLiteModule.kt: (106, 55): Type inference failed: Not enough information to infer parameter T in fun <reified T> arrayOfNulls(size: Int): Array<T?> Please specify it explicitly. FAILURE: Build failed with an exception. ``` Inspired to try this by expo/expo#15131 (comment) .
chrisbobbe
added a commit
to chrisbobbe/zulip-mobile
that referenced
this issue
Jan 25, 2022
Done by mostly following the changes to Expo's templates/expo-template-bare-minimum/ in expo/expo@9781212eb. Expo's description of the new infrastructure is at https://blog.expo.dev/whats-new-in-expo-modules-infrastructure-7a7cdda81ebc . They gave a migration guide that suggested making changes similar to these, but which didn't end up helping us understand why they were the right changes to make or what would happen if we made them. That guide is at https://github.com/expo/fyi/blob/main/expo-modules-migration.md . We've cleared up some of the mystery; see zulip#5133 and previous commits in this series. In this commit: - (Mostly follow expo/expo@9781212eb, as mentioned) - Also, upgrade all our `expo-*` direct dependencies so that they work with the new system. (We upgrade them minimally, to minimize having to think about possible unrelated breaking changes right now.) Details at zulip#5133 (comment) . - Remove android/app/src/main/java/com/zulipmobile/generated/BasePackageList.java, since its only import was removed. (Expo didn't have this file in version control, and the migration guide didn't mention the file.) - The Expo commit assumes that our project has done special setup for `expo-updates`, `expo-constants`, and `expo-splash-screen`. It makes changes to that setup, which we ignore since we don't use any of those. If we need them in the future, we'll just look up their current setup instructions at the time. - Don't add an empty Swift file. The migration guide says, "A blank Swift file must be created for native modules with Swift files to work correctly." With `find node_modules | grep .swift` in our project, I see that expo-modules-core and expo-web-browser have many Swift files in them, and I don't have any problems building or running the app without an empty Swift file. The template app doesn't add one either. - Don't add a Podfile.properties.json file. Like the changes in expo/expo@dbd384b22, this would have us put certain config values in some new Expo-branded variables. We're happy keeping them inline. - Update our value for `transformIgnorePatterns` in the Jest config, following an error when running Jest. Fixes: zulip#5133
chrisbobbe
added a commit
to chrisbobbe/zulip-mobile
that referenced
this issue
Jan 25, 2022
Done by mostly following the changes to Expo's templates/expo-template-bare-minimum/ in expo/expo@9781212eb. Expo's description of the new infrastructure is at https://blog.expo.dev/whats-new-in-expo-modules-infrastructure-7a7cdda81ebc . They gave a migration guide that suggested making changes similar to these, but it didn't end up helping us understand why they were the right changes to make or what would happen if we made them. That guide is at https://github.com/expo/fyi/blob/main/expo-modules-migration.md . We've cleared up some of the mystery; see zulip#5203 (comment) and previous commits in this series. In this commit: - (Mostly follow expo/expo@9781212eb, as mentioned) - Also, upgrade all our `expo-*` direct dependencies so that they work with the new system. (We upgrade them minimally, to minimize having to think about possible unrelated breaking changes right now.) Details at zulip#5203 (comment) . - Remove android/app/src/main/java/com/zulipmobile/generated/BasePackageList.java, since its only import was removed. (Expo didn't have this file in version control, and the migration guide didn't mention the file.) - The Expo commit assumes that our project has done special setup for `expo-updates`, `expo-constants`, and `expo-splash-screen`. It makes changes to that setup, which we ignore since we don't use any of those. If we need them in the future, we'll just look up their current setup instructions at the time. - Don't add an empty Swift file. The migration guide says, "A blank Swift file must be created for native modules with Swift files to work correctly." With `find node_modules | grep .swift` in our project, I see that expo-modules-core and expo-web-browser have many Swift files in them, and I don't have any problems building or running the app without an empty Swift file. The template app doesn't add one either. - Don't add a Podfile.properties.json file. Like the changes in expo/expo@dbd384b22, this would have us put certain config values in some new Expo-branded variables. We're happy keeping them inline. - Update our value for `transformIgnorePatterns` in the Jest config, following an error when running Jest. Fixes: zulip#5133
chrisbobbe
added a commit
to chrisbobbe/zulip-mobile
that referenced
this issue
Jan 25, 2022
Done by mostly following the changes to Expo's templates/expo-template-bare-minimum/ in expo/expo@9781212eb. Expo's description of the new infrastructure is at https://blog.expo.dev/whats-new-in-expo-modules-infrastructure-7a7cdda81ebc . They gave a migration guide that suggested making changes similar to these, but it didn't end up helping us understand why they were the right changes to make or what would happen if we made them. That guide is at https://github.com/expo/fyi/blob/main/expo-modules-migration.md . We've cleared up some of the mystery; see zulip#5203 (comment) and previous commits in this series. In this commit: - (Mostly follow expo/expo@9781212eb, as mentioned) - Also, upgrade all our `expo-*` direct dependencies so that they work with the new system. (We upgrade them minimally, to minimize having to think about possible unrelated breaking changes right now.) Details at zulip#5203 (comment) . - Remove android/app/src/main/java/com/zulipmobile/generated/BasePackageList.java, since its only import was removed. (Expo didn't have this file in version control, and the migration guide didn't mention the file.) - The Expo commit assumes that our project has done special setup for `expo-updates`, `expo-constants`, and `expo-splash-screen`. It makes changes to that setup, which we ignore since we don't use any of those. If we need them in the future, we'll just look up their current setup instructions at the time. - Don't add an empty Swift file. The migration guide says, "A blank Swift file must be created for native modules with Swift files to work correctly." With `find node_modules | grep .swift` in our project, I see that expo-modules-core and expo-web-browser have many Swift files in them, and I don't have any problems building or running the app without an empty Swift file. The template app doesn't add one either. - Don't add a Podfile.properties.json file. Like the changes in expo/expo@dbd384b22, this would have us put certain config values in some new Expo-branded variables. We're happy keeping them inline. - Update our value for `transformIgnorePatterns` in the Jest config, following an error when running Jest. - Some sub-dependency looks like it's changing the output of `generate-webview-js`. Examine that output (looks OK), and commit. Fixes: zulip#5133
chrisbobbe
added a commit
to chrisbobbe/zulip-mobile
that referenced
this issue
Jan 28, 2022
Like we did in 196a316. Changelog: https://github.com/JetBrains/kotlin/blob/master/ChangeLog.md#1610 This seems to fix the following error we'd see on running `tools/test native --all-files` after switching from Unimodules to Expo modules (zulip#5133): ``` e: /Users/chrisbobbe/dev/zulip-mobile/node_modules/expo-sqlite/android/src/main/java/expo/modules/sqlite/SQLiteModule.kt: (106, 55): Type inference failed: Not enough information to infer parameter T in fun <reified T> arrayOfNulls(size: Int): Array<T?> Please specify it explicitly. FAILURE: Build failed with an exception. ``` Inspired to try this by expo/expo#15131 (comment) .
chrisbobbe
added a commit
to chrisbobbe/zulip-mobile
that referenced
this issue
Jan 28, 2022
Done by mostly following the changes to Expo's templates/expo-template-bare-minimum/ in expo/expo@9781212eb. Expo's description of the new infrastructure is at https://blog.expo.dev/whats-new-in-expo-modules-infrastructure-7a7cdda81ebc . They gave a migration guide that suggested making changes similar to these, but it didn't end up helping us understand why they were the right changes to make or what would happen if we made them. That guide is at https://github.com/expo/fyi/blob/main/expo-modules-migration.md . We've cleared up some of the mystery; see zulip#5203 (comment) and previous commits in this series. In this commit: - (Mostly follow expo/expo@9781212eb, as mentioned) - Also, upgrade all our `expo-*` direct dependencies so that they work with the new system. (We upgrade them minimally, to minimize having to think about possible unrelated breaking changes right now.) Details at zulip#5203 (comment) . - Remove android/app/src/main/java/com/zulipmobile/generated/BasePackageList.java, since its only import was removed. (Expo didn't have this file in version control, and the migration guide didn't mention the file.) - The Expo commit assumes that our project has done special setup for `expo-updates`, `expo-constants`, and `expo-splash-screen`. It makes changes to that setup, which we ignore since we don't use any of those. If we need them in the future, we'll just look up their current setup instructions at the time. - Don't add an empty Swift file. The migration guide says, "A blank Swift file must be created for native modules with Swift files to work correctly." With `find node_modules | grep .swift` in our project, I see that expo-modules-core and expo-web-browser have many Swift files in them, and I don't have any problems building or running the app without an empty Swift file of our own. The template app doesn't add one either. - Don't add a Podfile.properties.json file. Like the changes in expo/expo@dbd384b22, this would have us put certain config values in some new Expo-branded variables. We're happy keeping the values inline. - Update our value for `transformIgnorePatterns` in the Jest config, following an error when running Jest. - Looks like some sub-dependency at its updated version wants to cause different output for `generate-webview-js`. Examine that output (looks OK), and commit. Fixes: zulip#5133
gnprice
pushed a commit
to chrisbobbe/zulip-mobile
that referenced
this issue
Jan 28, 2022
Like we did in 196a316. Changelog: https://github.com/JetBrains/kotlin/blob/master/ChangeLog.md#1610 This seems to fix the following error we'd see on running `tools/test native --all-files` after switching from Unimodules to Expo modules (zulip#5133): ``` e: /Users/chrisbobbe/dev/zulip-mobile/node_modules/expo-sqlite/android/src/main/java/expo/modules/sqlite/SQLiteModule.kt: (106, 55): Type inference failed: Not enough information to infer parameter T in fun <reified T> arrayOfNulls(size: Int): Array<T?> Please specify it explicitly. FAILURE: Build failed with an exception. ``` Inspired to try this by expo/expo#15131 (comment) .
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
—https://blog.expo.dev/whats-new-in-expo-modules-infrastructure-7a7cdda81ebc
The migration guide is here.
We'll want to do the "manual configuration" step because we have nontrivial custom code in
android/andios/and can't just clear those out and run a command to replace them.The text was updated successfully, but these errors were encountered: