fix(hosting/photo-storage): restore asset-canister recipe and fix canister-ID discovery - #1464
Open
marc0olo wants to merge 2 commits into
Open
fix(hosting/photo-storage): restore asset-canister recipe and fix canister-ID discovery#1464marc0olo wants to merge 2 commits into
marc0olo wants to merge 2 commits into
Conversation
…ister-ID discovery The example was broken twice over: 1. The switch to @dfinity/static-site (#1449) removed the legacy asset canister API (list/store/create_batch/commit_batch) that AssetManager from @icp-sdk/canisters/assets depends on — the app's entire purpose. Revert to @dfinity/asset-canister@v2.2.1, the only canister supporting programmatic uploads, with a comment explaining the exception (#1459). 2. The dfx-era canister-ID parsing broke on icp-cli's name-based frontend URLs (frontend.local.localhost) — Principal.fromText received "frontend.local" and threw at module load (white screen). Replace the hostname regex, ic0.app heuristic, and fetchRootKey() with the ic_env cookie pattern used by hello_world (safeGetCanisterEnv), which works on both URL forms and on mainnet. Also fix the README authorize command (canister is named frontend, not photo-storage) and point deploy instructions at the URL icp deploy prints. Verified end-to-end locally: gallery renders, uploads work after authorize, photos persist across reloads. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes the hosting/photo-storage example so programmatic uploads via AssetManager work again and the frontend reliably discovers its own canister ID regardless of whether the gateway URL is name-based or ID-based.
Changes:
- Revert
hosting/photo-storageback to the legacy@dfinity/asset-canister@v2.2.1recipe (required forAssetManagerAPIs). - Replace hostname-regex canister-ID parsing with
ic_envcookie discovery viasafeGetCanisterEnv()and usewindow.location.originfor agent host. - Update README deployment guidance and authorization command to target the
frontendcanister.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| hosting/photo-storage/src/App.jsx | Switches canister ID discovery to ic_env cookie and updates agent host/root key wiring. |
| hosting/photo-storage/README.md | Updates deploy URL guidance and fixes authorization command + II docs link. |
| hosting/photo-storage/icp.yaml | Reverts to the legacy asset-canister recipe with an explanatory comment linking the rationale/issue. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…k, fix README filename The ?canisterId= query-param fallback ran without the ic_env cookie and therefore without the local root key — a half-broken path nothing uses (the app is only ever served from the canister, which always sets the cookie; there is no dev-server flow). Require the cookie and fail with a clear error instead. Deliberately no fetchRootKey(): that is legacy behavior we no longer recommend — the root key comes from the certified ic_env cookie. Also fix the README warning to reference src/App.jsx (renamed from App.js in the Vite migration). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lwshang
approved these changes
Jul 31, 2026
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
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.
Problem
Two independent bugs, analyzed in #1458 / #1459:
@dfinity/static-siteexposes none of the legacy asset-canister API (list/store/create_batch/commit_batch) thatAssetManageruses — programmatic uploads, the example's purpose, cannot work on it (confirmed against certified-assets.did)."frontend.local"from icp-cli's name-based URL (frontend.local.localhost:8000) andPrincipal.fromTextthrewInvalid character: "."at module load → white screen.Fix
@dfinity/asset-canister@v2.2.1(interim per hosting/photo-storage broken on static-site recipe — revert to asset-canister interim, decide long-term fate #1459; comment inicp.yamlexplains the deliberate exception so the next recipe sweep doesn't re-migrate it).ic_envcookie pattern fromhello_world:safeGetCanisterEnv()providesPUBLIC_CANISTER_ID:frontend+IC_ROOT_KEY; agent useswindow.location.origin. Works on both name-based and ID-based URLs, locally and on mainnet.frontend(the actual canister name), deploy section points at the URLicp deployprints, II link → developer docs.Verification
Manual e2e (2026-07-30): gallery renders on
http://frontend.local.localhost:8000, upload works afterauthorize, photos persist across reloads. See #1459 for the long-term decision (keep-legacy / repurpose / retire).🤖 Generated with Claude Code