Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: upgrade Momento sdk dependencies in demos #80

Merged
merged 13 commits into from
Dec 2, 2024
Merged

Conversation

anitarua
Copy link
Collaborator

@anitarua anitarua commented Nov 26, 2024

Closes https://github.com/momentohq/dev-eco-issue-tracker/issues/1067

We recently added support for topics sequence page in our SDKs. We upgraded the Momento dependency for the main moderated chat web demo in #77.

This PR upgrades the Momento dependency in the remaining demos (react-native, android, ios, flutter).
Edit: bug fix was released in JS sdk so the react-native and web demo Momento dependencies were updated too.

Extra cleanup:

  • belatedly adds a gitignore for the Swift project and removes user-specific files.
  • caught a bug in the main web demo where messages are duplicated when switching between languages too soon (I think due to sequence page still being the same, so onItem was processing the most recent messages again). Fixed the bug in react-native, android, and ios as well. Flutter demo did not have the same issue.

Copy link

vercel bot commented Nov 26, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
moderated-chat ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 2, 2024 9:52pm

@@ -38,6 +38,7 @@
ignoresPersistentStateOnLaunch = "NO"
debugDocumentVersioning = "YES"
debugServiceExtension = "internal"
enableGPUValidationMode = "1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this do?

nand4011
nand4011 previously approved these changes Nov 27, 2024
@@ -1,5 +1,5 @@
// Top-level build file where you can add configuration options common to all sub-projects/modules.
plugins {
id("com.android.application") version "8.2.2" apply false
id("com.android.application") version "8.7.2" apply false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I tried running this in my copy of Android Studio, it prompted me to upgrade to Gradle 8.9. After I clicked the link to upgrade, I got the following error: The project is using an incompatible version (AGP 8.7.2) of the Android Gradle plugin. Latest supported version is AGP 8.2.2 Apparently only the very latest version of Android Studio has support for AGP 8.7, and the app builds and functions fine with the versions we were previously using. Unless you had a compelling reason to update this (and the distributionUrl in the gradle-wrapper.properties) I think we should keep them as they were.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay good to know, will change it back. I think these changed as a result of me clicking "yes" to a recommended update from Android Studio

```

Note: if you run into an error saying "principal class is nil because all fallbacks have failed", you can try turning off API validation in the settings as mentioned in this [github issue comment](https://github.com/flutter/flutter/issues/150227#issuecomment-2423291527).
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nand4011 the enableGPUValidationMode = "1" line was to handle this issue I ran into based on this comment. Without this, the app failed to build or run.

I ran it again this morning with this setting both on and off and it was fine though.

Would be good to get a second person to try the Swift demo and verify if the setting is actually needed

pgautier404
pgautier404 previously approved these changes Dec 2, 2024
Copy link
Contributor

@pgautier404 pgautier404 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@anitarua anitarua merged commit 5f78c02 into main Dec 2, 2024
3 checks passed
@anitarua anitarua deleted the update-deps branch December 2, 2024 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants