feat(mobile): add APP_VARIANT to make it easier to install multiple variants#2422
feat(mobile): add APP_VARIANT to make it easier to install multiple variants#2422xuatz wants to merge 4 commits intokarakeep-app:mainfrom
Conversation
…ariants - such as `production`, `development` and maybe `nightly` and `local`?
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a development variant flow: introduces IS_DEV in mobile app config to toggle name/identifiers, sets APP_VARIANT for dev builds in eas.json, adds mobile dev/release/clean scripts, and updates mobile setup docs and commands for dev workflows (iOS/Android). Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR adds
Confidence Score: 5/5
Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
apps/mobile/package.json
Outdated
| "android": "expo run:android", | ||
| "android:dev:release": "APP_VARIANT=development pnpm run android --variant release", | ||
| "ios": "expo run:ios", | ||
| "ios:dev:release": "echo 'i don't have an ios device to confirm if this works'", |
There was a problem hiding this comment.
style: placeholder echo command instead of actual implementation
| "ios:dev:release": "echo 'i don't have an ios device to confirm if this works'", | |
| "ios:dev:release": "APP_VARIANT=development pnpm run ios --configuration Release", |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/mobile/package.json
Line: 14:14
Comment:
**style:** placeholder echo command instead of actual implementation
```suggestion
"ios:dev:release": "APP_VARIANT=development pnpm run ios --configuration Release",
```
How can I resolve this? If you propose a fix, please make it concise.| alias: `pnpm --filter mobile clean:prebuild` | ||
| - why would I want to change the bundleIdentifier/package? | ||
| - so you can install `dev` variant, to try out your own build alongside your "production" karakeep app without uninstalling it | ||
| - you need to inject the env var`APP_VARIANT=development` |
There was a problem hiding this comment.
syntax: missing space between var and backtick
| - you need to inject the env var`APP_VARIANT=development` | |
| - you need to inject the env var `APP_VARIANT=development` |
Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/versioned_docs/version-v0.29.0/07-development/01-setup.md
Line: 145:145
Comment:
**syntax:** missing space between `var` and backtick
```suggestion
- you need to inject the env var `APP_VARIANT=development`
```
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/mobile/package.json`:
- Line 14: The npm script "ios:dev:release" contains an unescaped apostrophe in
the single-quoted string which breaks shell parsing; update the script value for
"ios:dev:release" to use double quotes around the string or escape the
apostrophe (e.g., "i don't have..." or 'i don'\''t have...') so the shell sees a
valid quoted string.
🧹 Nitpick comments (2)
package.json (1)
16-17: LGTM! Consider using the full package name for consistency.The scripts correctly delegate to the mobile workspace. However, other scripts in this file use the full scoped package name (e.g.,
@karakeep/web,@karakeep/db). For consistency, consider using@karakeep/mobile:- "android": "pnpm --filter mobile android", - "ios": "pnpm --filter mobile ios", + "android": "pnpm --filter `@karakeep/mobile` android", + "ios": "pnpm --filter `@karakeep/mobile` ios",Both forms work with pnpm, so this is a minor style suggestion.
docs/versioned_docs/version-v0.29.0/07-development/01-setup.md (1)
164-167: Consider removing personal anecdote from documentation.The phrase "my personal experience is that instead, the key is to run" is informal for project documentation. Consider rephrasing to be more direct:
📝 Suggested revision
-- You might need to add `APP_VARIANT=development` if you intend to run the dev variant, however, my personal experience is that instead, the key is to run `prebuild` with `APP_VARIANT=development` instead - `APP_VARIANT=development pnpm android` - or - `pnpm --filter mobile clean:prebuild:dev && pnpm android` +- For the dev variant, ensure you run `prebuild` with `APP_VARIANT=development`: + `pnpm --filter mobile clean:prebuild:dev && pnpm android`
| "android": "expo run:android", | ||
| "android:dev:release": "APP_VARIANT=development pnpm run android --variant release", | ||
| "ios": "expo run:ios", | ||
| "ios:dev:release": "echo 'i dont have an ios device to confirm if this works'", |
There was a problem hiding this comment.
heads up, i haven't tested it on ios ^^
Greptile found no issues!From now on, if a review finishes and we haven't found any issues, we will not post anything, but you can confirm that we reviewed your changes in the status check section. This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR". |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/versioned_docs/version-v0.29.0/07-development/01-setup.md`:
- Around line 179-182: Replace the verbose, informal paragraph about running the
Android dev variant with a concise authoritative instruction: remove anecdotal
language and the contradictory suggestion to set APP_VARIANT at runtime, and
instead state that you should run the prebuild-clean step followed by the
android command (use the existing npm script names referenced in the diff:
clean:prebuild:dev, prebuild and the pnpm android invocation); e.g., advise
running "pnpm --filter mobile clean:prebuild:dev && pnpm android" (omit
APP_VARIANT unless a specific build variant is required and, if so, state it
explicitly and consistently).
♻️ Duplicate comments (1)
docs/versioned_docs/version-v0.29.0/07-development/01-setup.md (1)
158-158: Fix spacing before inline code.Missing space between "var" and the backtick.
🔧 Proposed fix
- - you need to inject the env var `APP_VARIANT=development` + - you need to inject the env var `APP_VARIANT=development`
🧹 Nitpick comments (2)
docs/versioned_docs/version-v0.29.0/07-development/01-setup.md (2)
136-136: Consider a more professional heading.The triple question marks ("???") in the heading are unconventional for technical documentation. Consider a clearer alternative like "Quick Commands" or "TL;DR: Quick Start Commands".
📝 Suggested heading alternatives
-#### TLDR??? THIS IS TOO VERBOSE! +#### TL;DR: Quick Start Commandsor
-#### TLDR??? THIS IS TOO VERBOSE! +#### Quick Commands
188-188: Clarify the release build concept more directly.The phrase "It might be slightly confusing, but..." is unnecessarily tentative. State the concept clearly and confidently.
✍️ Suggested rewording
-It might be slightly confusing, but if you want to install the `dev` variant of the app, but you want it to be a "release" build; does not use the dev server, you can do the following: +To install the `dev` variant as a release build (standalone, without the dev server), use the following commands:
| - You might need to add `APP_VARIANT=development` if you intend to run the dev variant, however, my personal experience is that instead, the key is to run `prebuild` with `APP_VARIANT=development` instead. But after you already prebuild properly, then you can just run run expo | ||
| `APP_VARIANT=development pnpm android` | ||
| or | ||
| `pnpm --filter mobile clean:prebuild:dev && APP_VARIANT=development pnpm android` |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Simplify and clarify the Android setup instructions.
The current explanation is verbose, contradictory, and uses informal language ("my personal experience"). The guidance should be clear and authoritative.
📚 Proposed simplification
-- You might need to add `APP_VARIANT=development` if you intend to run the dev variant, however, my personal experience is that instead, the key is to run `prebuild` with `APP_VARIANT=development` instead. But after you already prebuild properly, then you can just run run expo
- `APP_VARIANT=development pnpm android`
- or
- `pnpm --filter mobile clean:prebuild:dev && APP_VARIANT=development pnpm android`
+- For the dev variant, ensure you've run prebuild with `APP_VARIANT=development` first:
+ ```sh
+ pnpm --filter mobile clean:prebuild:dev && pnpm android
+ ```Note: This aligns with the TLDR section above and provides clear, non-contradictory guidance.
🧰 Tools
🪛 LanguageTool
[style] ~179-~179: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...ted on the emulator/device. - You might need to add APP_VARIANT=development if you in...
(REP_NEED_TO_VB)
🤖 Prompt for AI Agents
In `@docs/versioned_docs/version-v0.29.0/07-development/01-setup.md` around lines
179 - 182, Replace the verbose, informal paragraph about running the Android dev
variant with a concise authoritative instruction: remove anecdotal language and
the contradictory suggestion to set APP_VARIANT at runtime, and instead state
that you should run the prebuild-clean step followed by the android command (use
the existing npm script names referenced in the diff: clean:prebuild:dev,
prebuild and the pnpm android invocation); e.g., advise running "pnpm --filter
mobile clean:prebuild:dev && pnpm android" (omit APP_VARIANT unless a specific
build variant is required and, if so, state it explicitly and consistently).
1 participant
Summary
app.hoarder.hoardermobileorapp.hoarder.hoardermobile.devbased onprocess.env.APP_VARIANTDetails
Added env var
APP_VARIANT, that is entirely meant for developers, not for users, to make it more convenient to install multiple instances of the mobile app on their devices.Currently only supports production build (default) and dev build (process.env.APP_VARIANT === "development"). Potentially it could be extended to include other variants such as
APP_VARIANT=(nightly|local)in future.