React Router authenticate.admin helper + Resource Picker UI mock - #36
Open
fazlulkabir34 wants to merge 31 commits into
Open
React Router authenticate.admin helper + Resource Picker UI mock#36fazlulkabir34 wants to merge 31 commits into
fazlulkabir34 wants to merge 31 commits into
Conversation
- Add withMockBridgeAdminAuthForReactRouter under @getverdict/mock-bridge/react-router - Wire package.json exports for ., ./auth, ./react-router, ./client - Optional peerDependencies for @shopify/shopify-app-react-router and @shopify/shopify-api - Document SHOPIFY_APP_REACT_ROUTER, BACKEND_INTEGRATION caveat, README link - tsconfig paths for Shopify package subpath typings under classic moduleResolution
…ithMockBridgeAdminAuthForReactRouter
…kage exports - Introduced withMockBridgeAdminAuthForReactRouter for enhanced admin authentication in React Router apps. - Updated package.json to remove deprecated react-router export and streamline exports. - Enhanced documentation to reflect changes in integration instructions for Shopify App React Router. - Adjusted type exports for better clarity and consistency.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Removed redundant mention of `@getverdict/mock-bridge/auth` in the Shopify App React Router section.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Feature/react router admin auth
…bevycommerce/mock-bridge into feature/react-router-admin-auth
Feature/react router admin auth
chore: update package-lock.json and tsconfig.json
…AdminAuthForReactRouter imports
- Added ResourcePicker component for selecting products, variants, and collections. - Updated App component to include ResourcePicker. - Enhanced mock bridge to handle resource picker actions. - Updated package.json and package-lock.json to include new dependencies for React and related types. - Modified README.md to reflect the new ResourcePicker functionality. - Updated tsconfig.json to include new type definitions for resource picker.
…d functions - Enhanced ResourcePickerCatalogResponse to include variants. - Updated getDefaultResourcePickerCatalog to return variants using flattenVariants. - Modified mergeResourcePickerCatalog to handle variants in the merge process. - Adjusted server logic to utilize the new variants structure.
- Removed unnecessary useCallback for toggleId in ResourcePickerModal. - Simplified onChange handlers for buttons in ResourcePickerModal. - Introduced fetchCatalog function to centralize catalog fetching logic and improve error handling. - Updated closedPickerState to include loading state. - Enhanced getFeatureStore to throw an error if a feature is not found. - Refactored flattenVariants function for better readability and performance.
Feature/react router admin auth
… improvements - Added focus management for the search input in ResourcePickerModal to improve accessibility. - Implemented keyboard navigation within the dialog to enhance user experience. - Updated the ResourcePicker state to include selectionRowById for better tracking of selected items. - Refactored fetchCatalog to handle aborting previous requests and improved error handling. - Adjusted the mock bridge to support new features and ensure robust communication with the resource picker.
Feature/resource picker
Syncs 5 months of upstream dependabot bumps into the fork, which had diverged since 2eb5394 (upstream PR ctrlaltdylan#24) with the React Router admin auth helper and the mock ResourcePicker. Upstream brings (all dependency bumps, no source changes): - follow-redirects 1.15.11 -> 1.16.0 - admin-frame: vite 5.4 -> 7.3.1, esbuild 0.27.3 - uuid 9.0.1 -> 14.0.0 Conflict resolution (package.json dependencies): Upstream's uuid 14 is ESM-only: "type": "module" with no `require` condition in its exports map. This package compiles to CommonJS ("module": "commonjs"), so the published dist does `require('uuid')`, which only works on Node >= 22.12 via require(esm). Since package.json declares "engines": { "node": ">=18.0.0" }, taking uuid 14 verbatim would break consumers on Node 18/20 with ERR_REQUIRE_ESM. uuid 9 is also deprecated upstream ("uuid@10 and below is no longer supported"), so it is not a durable resting place either. Resolved to uuid ^11.1.1, which ships a real CommonJS build (exports["."].node.require -> ./dist/cjs/index.js), is not deprecated, and declares no engines floor. The only API used is `v4` (src/auth/token-generator.ts, app-bridge/src/invokeFeature.ts), which is unchanged across 9 -> 11. Also drops @types/uuid ^9.0.7: uuid 11 ships its own .d.ts and tsc passes without it. Verified: tsc --noEmit clean, full build passes on vite 7, `require('uuid').v4()` works at runtime, app-bridge bundle rebuilt.
Vite 7.3.1 (pulled in by the upstream sync) declares engines.node ^20.19.0 || >=22.12.0, but nothing in the repo stated a Node requirement anywhere. Deliberately does NOT raise the root package's engines.node from >=18.0.0. That field is the runtime contract for consumers of the published package, and Node 18 genuinely satisfies it: no runtime dependency has a floor above 18 (the highest is http-proxy-middleware at >=18.0.0), and the published tarball ships only prebuilt dist/, assets/, admin-frame/dist/ and app-bridge/dist/, so consumers never install or execute Vite. Raising it to 22 would emit EBADENGINE -- or hard-fail under engine-strict -- for every Node 18/20 consumer, over a build tool they never run. Instead the constraint is recorded where it is actually true: - admin-frame/package.json gains engines.node matching Vite's own range. admin-frame is "private": true, so this constrains contributors only and has no effect on published consumers. - README documents both numbers separately: Node 18+ to use the package, ^20.19.0 || >=22.12.0 to build from source (including git-ref installs, where `prepare` runs the full build). - CLAUDE.md notes the build requirement next to the build commands. Verified: full `npm run build` passes, root engines.node unchanged.
app-bridge/src/invokeFeature.ts imports `uuid`, but app-bridge/package.json
declared no uuid at all -- only @shopify/app-bridge-types, esbuild and
typescript. The import resolved purely by accident, via hoisting from the
root node_modules, and app-bridge/package-lock.json had zero uuid entries.
This is a real failure, not a theoretical one. Building app-bridge in
isolation from the root (its src + package.json + lockfile, npm install,
npm run build) fails:
X [ERROR] Could not resolve "uuid"
src/invokeFeature.ts:1:29:
1 | import { v4 as uuidv4 } from 'uuid';
The same isolated build succeeds after this change.
Declared in devDependencies rather than dependencies: app-bridge is never
installed as a package by anyone -- esbuild inlines uuid into a self-contained
IIFE, and only the built dist/index.js ships (via the root package's `files`).
uuid is therefore a build-time input, exactly like the esbuild and typescript
entries beside it. An --omit=dev install could not build this package anyway,
since esbuild itself is a devDependency, so nothing is lost by the placement.
Pinned to ^11.1.1 to match the root, which moved to uuid 11 in the upstream
sync for CommonJS compatibility.
Verified: isolated standalone build fails before / passes after, full
`npm run build` passes, and the emitted bundle is byte-identical.
chore: sync upstream ctrlaltdylan/mock-bridge (resolve uuid ESM conflict)
fix: declare uuid as an explicit dependency of app-bridge
The patched `window.fetch` attaches `Authorization: Bearer <currentSessionToken>` to every request, but `currentSessionToken` was only ever assigned inside the SESSION_TOKEN_RESPONSE handler — so it stayed null until the app called `shopify.idToken()`. App Bridge v4 apps typically never call it, because the real App Bridge hands them an already-authenticated fetch. Every data request therefore went out unauthenticated. In an embedded Remix or React Router app that means the host bounces client-side loader fetches to the session-token page: navigation lands on the app's own login route, and on-mount API calls try to parse that HTML as JSON. Seed `currentSessionToken` from the `id_token` the host already puts in the iframe URL, and start the refresh loop for the `shopify` global rather than only inside `createApp()` — the legacy v2 entry point a v4 app never calls. Also adds `sessionTokenTtlSeconds` so tokens can outlive a long-running test. The default stays 60s to match real Shopify, and the refresh interval derives from it (half the TTL, clamped to 5–30s). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Yarn 1 does not run a git dependency's `prepare` script. It clones the repo as-is, so `build:admin-frame` never runs, `admin-frame/dist` is absent, and the `express.static()` mount serves nothing — the embedded iframe renders blank. npm consumers are unaffected: npm does run `prepare` and packs per `files`. Committing the output makes both package managers behave identically and removes a build-on-install from consumer CI. Un-ignores `dist/` at the root too, matching the tracked state of `dist/` and `app-bridge/dist/`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rmup fix: hold a session token before the app asks for one, + make yarn git installs work
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
This branch brings two main upgrades to mock-bridge: first-class support for Shopify App React Router admin authentication in tests, and a fully mocked shopify.resourcePicker flow with an admin-frame modal, catalog API, and stronger bridge integration—so automated browser tests and MCP-style tooling can exercise picker UX without hitting Shopify.
What’s included
React Router admin auth — withMockBridgeAdminAuthForReactRouter (and related exports/build artifacts) so apps using authenticate.admin(request) can run against the mock bridge with clearer wiring and types.
Resource Picker — Replaces the previous stub with a mock modal UI in the admin frame, store/state for open/close/search/selection, variant-aware catalog behavior, and improvements around focus management and error handling.
Server/catalog integration — Mock catalog backed by /api/resource-picker-catalog (documented); customizable via resourcePickerCatalog on the server where applicable.
App Bridge layer — Updates to feature wiring (invokeFeature, resource-picker bridge path, loading tweaks) so the picker behaves consistently with the rest of the mock.
Docs — New guides: docs/SHOPIFY_APP_REACT_ROUTER.md, docs/RESOURCE_PICKER.md; README feature matrix updated (resource picker marked as mock UI); small related edits to backend integration and Remix docs.