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: update core and ipc to v2.0.0 #796

Merged
merged 6 commits into from
Oct 17, 2024
Merged

chore: update core and ipc to v2.0.0 #796

merged 6 commits into from
Oct 17, 2024

Conversation

achou11
Copy link
Member

@achou11 achou11 commented Oct 15, 2024

Towards #622, #623, and #624

  • Updates @comapeo/core and @comapeo/ipc to 2.0.0.
  • App changes:
    • Removes support for embedded static maps. This is related to inserting an asar-formatted file into a highly specific directory on the android device, which is the "legacy" approach for custom maps inherited from Mapeo. This was more of a stop-gap feature that wasn't expected to be heavily used anyways, and we made the decision to drop support for this entirely in core. feat: add support for custom offline maps #318 will introduce an improvement to that feature that is much more reliable and easier to use.
    • Removes some map server boilerplate code since that is now handled within the MapeoManager itself.

Online default style:

image

Offline fallback:

image

@achou11 achou11 requested a review from ErikSin October 15, 2024 18:19
Copy link
Member Author

Choose a reason for hiding this comment

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

confirming that the hypercore version does not unexpectedly change here (cd src/backend && npm list hypercore):

[email protected] /Users/andrewchou/GitHub/digidem/comapeo-mobile/src/backend
└─┬ @comapeo/[email protected]
  ├─┬ [email protected]
  │ └── [email protected] deduped
  └── [email protected]

Copy link
Member Author

Choose a reason for hiding this comment

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

No longer needed since updated core includes this (see digidem/comapeo-core@d4b9904)

Copy link
Member Author

@achou11 achou11 Oct 15, 2024

Choose a reason for hiding this comment

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

no longer needed since updated core removes support for the static maps approach, which relied on using asar files (see digidem/comapeo-core@f21a675#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519L157)

src/backend/src/app.js Outdated Show resolved Hide resolved
}) {
log('Starting app...')
log(`Device version is ${version}`)

const privateStorageDir = rnBridge.app.datadir()
const dbDir = join(privateStorageDir, DB_DIR_NAME)
const indexDir = join(privateStorageDir, CORE_STORAGE_DIR_NAME)
const staticStylesDir = join(sharedStoragePath, 'styles')
Copy link
Member Author

Choose a reason for hiding this comment

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

the removal of this means that devices that have installed the app previously will have a lingering directory, which shouldn't be a problem but might be something worth documenting somehow?

Copy link
Member Author

@achou11 achou11 left a comment

Choose a reason for hiding this comment

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

While working on #758 to build on top of this PR, I realized that we can get rid of some more boilerplate + complexity. See 48a9193 for relevant changes.

"@dev-plugins/react-navigation": "^0.0.6",
"@dr.pogodin/react-native-fs": "^2.24.1",
Copy link
Member Author

Choose a reason for hiding this comment

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

no longer used at all given the removal of the external shared directory path

@@ -56,49 +50,27 @@ process.on('exit', (code) => {
* @param {string} [options.version] Device Version
* @param {Buffer} options.rootKey
* @param {string} options.migrationsFolderPath
* @param {string} options.sharedStoragePath Path to app-specific external file storage folder
Copy link
Member Author

Choose a reason for hiding this comment

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

we no longer need this option because we're strictly going to use the app's "internal" storage - AKA the document directory - to store the custom maps. The path to this directory is already available via nodejs-mobile e.g. const privateStorageDir = rnBridge.app.datadir()

The setup for this will be done in the other PR

@achou11 achou11 requested review from cimigree and removed request for ErikSin October 16, 2024 21:38
Copy link
Contributor

@cimigree cimigree left a comment

Choose a reason for hiding this comment

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

Passes end to end tests but that is all I can speak to.

@achou11 achou11 merged commit fe74e7e into develop Oct 17, 2024
3 checks passed
@achou11 achou11 deleted the ac/update-core-2.0.0 branch October 17, 2024 19:07
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.

2 participants