Escape store links from in-app browsers - #4274
Draft
pepeladeira wants to merge 7 commits into
Draft
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an in-app browser escape flow for Instagram, Facebook, and TikTok browser contexts. The middleware creates encoded escape URLs, and the escape page renders localized messaging, branding, destination details, and browser or copy actions. ChangesIn-app browser escape
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Request
participant LinkMiddleware
participant EscapePage
participant Prisma
participant ActionButton
Request->>LinkMiddleware: request store destination
LinkMiddleware->>LinkMiddleware: detect supported in-app browser
LinkMiddleware-->>Request: rewrite to encoded escape URL
Request->>EscapePage: load escape URL
EscapePage->>Prisma: resolve optional link configuration
Prisma-->>EscapePage: return configuration or destination fallback
EscapePage->>ActionButton: provide destination and copy URLs
ActionButton-->>Request: open external browser or copy URL
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
apps/web/app/in-app-browser/[url]/page.tsx (2)
36-40: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winFalling back to
"instagram"for an unknownsourceis misleading.Line 81 coerces any unrecognized
sourcevalue to"instagram". The page then emits aninstagram://extbrowser/scheme and auto-navigates to it at Line 151. On a device without Instagram installed, that navigation fails and the user sees nothing happen.Treat an unknown
sourceas "no scheme available" instead, so the page renders the copy-link fallback.♻️ Proposed change to the source fallback
- const source: InAppBrowserSource = VALID_SOURCES.has( - rawSource as InAppBrowserSource, - ) - ? (rawSource as InAppBrowserSource) - : "instagram"; + const source: InAppBrowserSource | null = VALID_SOURCES.has( + rawSource as InAppBrowserSource, + ) + ? (rawSource as InAppBrowserSource) + : null;
getExtBrowserSchemethen needs to acceptnulland returnnullfor it.Also applies to: 77-81
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/app/in-app-browser/`[url]/page.tsx around lines 36 - 40, The source fallback logic (around the VALID_SOURCES reference) currently coerces unrecognized source values to "instagram", which triggers a misleading instagram:// scheme navigation that fails if the app isn't installed. Instead of defaulting unknown sources to a valid scheme, treat them as null to represent "no scheme available". Update the getExtBrowserScheme function to accept null as input and return null for it, so the page falls back to rendering the copy-link UI for unrecognized sources rather than attempting a failed navigation.
105-130: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRemove the duplicate Prisma lookup from the escape page.
The in-app browser rewrite only passes
domainandkeythrough search params, soapps/web/app/in-app-browser/[url]/page.tsxresolvesshortLinkandshortDomain.deepviewDataagain. Move those fields into the rewrite state, or avoid re-fetching when they are already available.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/app/in-app-browser/`[url]/page.tsx around lines 105 - 130, The Prisma findUnique query around the domain_key lookup is redundantly fetching shortLink and shortDomain.deepviewData even though the rewrite already passes domain and key through search params. Either include shortLink and deepviewData in the rewrite state to make them available without re-fetching, or conditionally skip the findUnique query if those fields are already present. Preserve the decodeLinkIfCaseSensitive logic for decoding the resolved link data.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/app/in-app-browser/`[url]/action-button.tsx:
- Around line 38-46: Update the button styling in the action-button component so
its text color remains readable when buttonStyle.backgroundColor is customized.
Derive the label color from the configured background or apply a
buttonStyle.color override, while preserving the existing white-text fallback
when no custom styling is provided.
- Around line 48-53: The copy button in the action-button component fails
silently when the Clipboard API is unavailable, leaving the user with no
feedback. Update the onClick handler for the Button to pass throwOnError to
copyToClipboard (or configure useCopyToClipboard with throwOnError), then add
error state tracking alongside the existing copied state to capture and display
failures. When an error occurs, show an error message or visual indicator to the
user so they know the clipboard action failed, or alternatively render the
copyUrl as selectable text below the button so users can manually select and
copy the URL when the Clipboard API is unavailable.
In `@apps/web/app/in-app-browser/`[url]/page.tsx:
- Around line 54-66: The TikTok path in getExtBrowserScheme currently returns
null, causing InAppBrowserActionButton to navigate within the embedded webview
while still displaying the “Open in App Store” action. Update the related
action-button behavior so the primary action is hidden when extBrowserScheme is
null, while preserving the copy-link fallback.
- Around line 93-95: Remove the decodeURIComponent call from the key assignment
in the page component, using searchParams.key directly when present and
preserving undefined when absent. Keep the existing key lookup flow unchanged so
literal percent characters are handled safely and the original decoded value
reaches findUnique.
In `@apps/web/app/in-app-browser/`[url]/translations.ts:
- Around line 25-26: Update the localized description and openInStore templates
in the French, Portuguese, and German translation entries to avoid fixed
articles that conflict with either storeName value. Remove or replace the
surrounding article phrasing while preserving the {storeName} interpolation and
the intended meaning for both Google Play and App Store.
In `@apps/web/lib/middleware/link.ts`:
- Around line 420-433: Consolidate destination selection and final URL
computation in the surrounding link middleware: reuse the existing precedence
logic from the branches at Lines 472, 536, and 599 instead of maintaining the
duplicate destinationForEscape chain. Compute getFinalUrl once for the resolved
destination, then pass the shared destination and final URL through those
branches so each request avoids repeated resolution and URL generation.
- Around line 435-469: Update the inAppBrowserSource escape branch before its
early return to call cacheDeepLinkClickData for eligible App Store or Play Store
destinations when skip_deeplink_preview is absent. Pass the main destination URL
rather than escapeFinalUrl so deferred deep-link tracking can recover the
original application URL.
---
Nitpick comments:
In `@apps/web/app/in-app-browser/`[url]/page.tsx:
- Around line 36-40: The source fallback logic (around the VALID_SOURCES
reference) currently coerces unrecognized source values to "instagram", which
triggers a misleading instagram:// scheme navigation that fails if the app isn't
installed. Instead of defaulting unknown sources to a valid scheme, treat them
as null to represent "no scheme available". Update the getExtBrowserScheme
function to accept null as input and return null for it, so the page falls back
to rendering the copy-link UI for unrecognized sources rather than attempting a
failed navigation.
- Around line 105-130: The Prisma findUnique query around the domain_key lookup
is redundantly fetching shortLink and shortDomain.deepviewData even though the
rewrite already passes domain and key through search params. Either include
shortLink and deepviewData in the rewrite state to make them available without
re-fetching, or conditionally skip the findUnique query if those fields are
already present. Preserve the decodeLinkIfCaseSensitive logic for decoding the
resolved link data.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9bcbdeeb-5283-41aa-82c4-475fee916995
📒 Files selected for processing (5)
apps/web/app/in-app-browser/[url]/action-button.tsxapps/web/app/in-app-browser/[url]/page.tsxapps/web/app/in-app-browser/[url]/translations.tsapps/web/lib/middleware/link.tsapps/web/lib/middleware/utils/detect-in-app-browser.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/app/in-app-browser/[url]/page.tsx (1)
26-30: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused static rendering config, or document why it stays.
InAppBrowserEscapePagereads request data withheaders()beforegetLanguage()anduserAgent(). That makes the route dynamic, sorevalidate = falseandgenerateStaticParams()do not affect rendering. Keep the config if a specific reason applies; otherwise remove it to avoid a futureheaders()removal accidentally enabling stale static cache.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/app/in-app-browser/`[url]/page.tsx around lines 26 - 30, Remove the unused revalidate export and generateStaticParams function near InAppBrowserEscapePage, since headers() makes this route dynamic; only retain them if you add documentation explaining a specific intentional purpose.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/web/app/in-app-browser/`[url]/page.tsx:
- Around line 26-30: Remove the unused revalidate export and
generateStaticParams function near InAppBrowserEscapePage, since headers() makes
this route dynamic; only retain them if you add documentation explaining a
specific intentional purpose.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4cf66a69-b8c2-4ab2-ba63-e40ed7c77596
📒 Files selected for processing (3)
apps/web/app/in-app-browser/[url]/action-button.tsxapps/web/app/in-app-browser/[url]/page.tsxapps/web/lib/middleware/link.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/app/in-app-browser/[url]/action-button.tsx
Collaborator
Author
|
@coderabbitai full review. any breaking changes? |
Contributor
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
git diff --stat HEAD^ HEAD
git diff --find-renames -- \
'apps/web/lib/middleware/link.ts' \
'apps/web/lib/middleware/utils/detect-in-app-browser.ts' \
'apps/web/app/in-app-browser/[url]/action-button.tsx' \
'apps/web/app/in-app-browser/[url]/page.tsx' \
'apps/web/app/in-app-browser/[url]/translations.ts'
printf '\n--- Relevant symbols and references ---\n'
ast-grep outline apps/web/lib/middleware/link.ts --items all
ast-grep outline apps/web/lib/middleware/utils/detect-in-app-browser.ts --items all
ast-grep outline 'apps/web/app/in-app-browser/[url]/page.tsx' --items all
rg -n -C 3 '\b(shouldEscapeInAppBrowser|getInAppBrowserEscapeUrl|InAppBrowserEscapePage|InAppBrowserActionButton)\b' apps/web
printf '\n--- Related test files ---\n'
fd -t f . apps/web | rg -i '(in-app-browser|detect-in-app-browser|middleware.*link|link.*middleware).*(test|spec)|\.(test|spec)\.[^.]+$' || trueLength of output: 14052 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- PR commits and changed paths ---'
git log --oneline HEAD^..HEAD
git diff --name-status HEAD^ HEAD
printf '%s\n' '--- Current LinkMiddleware redirect section ---'
sed -n '300,510p' apps/web/lib/middleware/link.ts
printf '%s\n' '--- Parent LinkMiddleware redirect section ---'
git show HEAD^:apps/web/lib/middleware/link.ts | sed -n '300,510p'
printf '%s\n' '--- In-app browser helpers ---'
cat -n apps/web/lib/middleware/utils/detect-in-app-browser.ts
printf '%s\n' '--- URL/store helpers ---'
cat -n apps/web/lib/middleware/utils/is-ios-app-store-url.ts
cat -n apps/web/lib/middleware/utils/is-google-play-store-url.ts
cat -n apps/web/lib/middleware/utils/get-final-url.ts
printf '%s\n' '--- Escape page and action component ---'
sed -n '1,340p' 'apps/web/app/in-app-browser/[url]/page.tsx'
sed -n '1,220p' 'apps/web/app/in-app-browser/[url]/action-button.tsx'
printf '%s\n' '--- Effective diff with context ---'
git diff --find-renames -U80 HEAD^ HEAD -- \
'apps/web/lib/middleware/link.ts' \
'apps/web/app/in-app-browser/[url]/page.tsx' \
'apps/web/app/in-app-browser/[url]/action-button.tsx'Length of output: 57039 🏁 Script executed: #!/bin/bash
set -euo pipefail
base="$(git merge-base HEAD main)"
printf 'Merge base: %s\n' "$base"
printf '%s\n' '--- Complete PR changed paths ---'
git diff --name-status "$base" HEAD
printf '%s\n' '--- Complete PR diff summary ---'
git diff --stat "$base" HEAD
printf '%s\n' '--- Complete PR diff: in-app-browser and middleware ---'
git diff -U50 "$base" HEAD -- \
'apps/web/lib/middleware/link.ts' \
'apps/web/lib/middleware/utils/detect-in-app-browser.ts' \
'apps/web/app/in-app-browser/[url]/page.tsx' \
'apps/web/app/in-app-browser/[url]/action-button.tsx' \
'apps/web/app/in-app-browser/[url]/translations.ts'
printf '%s\n' '--- Middleware and route configuration references ---'
rg -n -C 4 'LinkMiddleware|matcher|in-app-browser|/deeplink/' \
apps/web/middleware.ts apps/web/next.config.* apps/web 2>/dev/null || true
printf '%s\n' '--- Target branch versions, when files exist ---'
for f in \
'apps/web/lib/middleware/utils/detect-in-app-browser.ts' \
'apps/web/app/in-app-browser/[url]/page.tsx' \
'apps/web/app/in-app-browser/[url]/action-button.tsx' \
'apps/web/app/in-app-browser/[url]/translations.ts'
do
if git cat-file -e "$base:$f" 2>/dev/null; then
printf '\n--- %s at merge base ---\n' "$f"
git show "$base:$f" | sed -n '1,360p'
fi
done
printf '%s\n' '--- URL construction edge cases ---'
python3 - <<'PY'
from urllib.parse import quote
for u in [
"https://play.google.com/store/apps/details?id=com.example#reviews",
"https://apps.apple.com/us/app/example/id123?pt=1#privacy",
"https://play.google.com:8443/store/apps/details?id=com.example",
]:
print(u)
print("encoded path:", quote(u, safe=""))
PYLength of output: 45658 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- Middleware dispatch and application-route exclusions ---'
sed -n '1,115p' apps/web/middleware.ts
sed -n '1,95p' apps/web/lib/middleware/app.ts
printf '%s\n' '--- Domain and request parsing behavior ---'
rg -n -C 3 '\bAPP_DOMAIN\b|function isDubDomain|const isDubDomain|export.*isDubDomain' \
packages apps/web 2>/dev/null || true
cat -n apps/web/lib/middleware/utils/parse.ts
printf '%s\n' '--- Existing test conventions for middleware utilities ---'
fd -t f -e ts -e tsx apps/web | rg '(test|spec)\.(ts|tsx)$' | head -80
rg -n -C 2 'NextRequest|NextFetchEvent|LinkMiddleware|userAgent\(' \
apps/web/tests apps/web/lib --glob '*.{test,spec}.ts' 2>/dev/null || true
printf '%s\n' '--- Package scripts and test tooling declarations ---'
for f in package.json apps/web/package.json; do
if [ -f "$f" ]; then
printf '\n--- %s ---\n' "$f"
sed -n '1,220p' "$f"
fi
doneLength of output: 64519
I found no breaking public API or redirect-contract change. The PR intentionally changes behavior for Instagram, Facebook, and TikTok in-app browsers when the selected destination is an App Store or Google Play URL. These requests now render the escape page instead of continuing through the normal store-link flow. For all other requests, the existing destination priority remains unchanged:
The implementation preserves click recording, cookies, tracking parameters, and deferred deep-link caching before it enters the escape flow. I did not identify a blocking code defect. The main release risk is device-specific behavior for the Instagram, Facebook, and Android ✅ Action performedFull review finished. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/app/in-app-browser/`[url]/translations.ts:
- Around line 86-104: Update the language validation in getLanguage to accept
only translation codes that are own properties of translations, using
Object.prototype.hasOwnProperty.call or an equivalent valid-code Set. Preserve
the existing fallback to "en" and return behavior for supported language codes.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 512a4df5-220b-4b9f-be5a-91057bbbdaac
📒 Files selected for processing (5)
apps/web/app/in-app-browser/[url]/action-button.tsxapps/web/app/in-app-browser/[url]/page.tsxapps/web/app/in-app-browser/[url]/translations.tsapps/web/lib/middleware/link.tsapps/web/lib/middleware/utils/detect-in-app-browser.ts
pepeladeira
marked this pull request as draft
August 4, 2026 21:55
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit