-
Notifications
You must be signed in to change notification settings - Fork 354
docs: add critical security and dependency resolution guidelines to CLAUDE.md #3137
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
Open
kdaviduik
wants to merge
20
commits into
main
Choose a base branch
from
kd-update-CLAUDE
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
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
## Summary Updated both Storefront API and Customer Account API from version 2025-04 to 2025-07 ## Changes - Updated version constants in hydrogen-react and hydrogen packages - Regenerated GraphQL types and schemas for both APIs - Updated all hardcoded version references in documentation and tests - Regenerated skeleton template types - Created 6 GitHub issues for tracking API changes implementation ## API Changes Tracked - #3076: Subscription discount data in Customer Account API - #3077: Cart warnings for non-applicable discount codes - #3078: BUYER_CANNOT_PURCHASE_FOR_COMPANY_LOCATION cart error - #3079: New order filter options in Customer Account API - #3080: Imperial units in UnitPriceMeasurement - #3081: Selling plan error handling in Cart mutations ## Validation - TypeScript: ✅ No errors - Tests: ✅ All passing - Lint:⚠️ 2 warnings (non-blocking import duplicates) - Build: ✅ Successful
- Update from ~3.80.4 to ~3.83.3 - Include in changeset documentation
…3091) * fix: Complete React Router 7 migration for Express example The Express example was partially migrated to React Router 7 but missing critical configuration and dependencies, causing CI failures. Changes: - Add react-router.config.ts with proper preset and app directory configuration - Fix vite.config.ts to call reactRouter() without arguments (RR7 API change) - Add @react-router/dev as devDependency for TypeScript types - Update entry.server.tsx to remove unnecessary @remix-run/web-fetch import (Node 20+ provides native Response globally) - Fix environment variable loading using getEnv() helper - Add .gitignore for React Router generated type files - Update tsconfig.json to exclude generated directories The example now builds successfully with React Router 7's framework mode. * fix: Fix CLI build checks and TypeScript configuration Multiple CLI-specific issues were causing CI failures after recent refactoring. Changes: - Fix build-check.mjs to expect .jsx extensions (tsup outputs .tsx→.jsx) - Exclude assets/routes from TypeScript checking (template files for user projects) - Align ast-grep to version 0.33.0 to match @shopify/cli's bundled version - Fix isHydrogenMonorepo() detection to work in both dev and npm package contexts - Add 'as const' assertions for ast-grep 0.33.0 API compatibility Background: - Template files were moved to assets/routes in May 2024 and inadvertently included in TypeScript scope - ast-grep 0.34.1 conflicted with @shopify/cli's bundled 0.33.0 * fix: Fix CI workflow and monorepo build orchestration The CI workflow and Turbo configuration had multiple issues causing intermittent build failures, particularly the "No lockfile found" error. Changes: - Add SHOPIFY_HYDROGEN_FLAG_LOCKFILE_CHECK=false to typecheck step in CI (was only on build:all, but typecheck also triggers builds via Turbo) - Add SHOPIFY_HYDROGEN_FLAG_LOCKFILE_CHECK to turbo.json globalEnv - Add explicit build dependencies for all examples in turbo.json - Remove docs-preview from workspaces to prevent React Router version conflicts - Regenerate package-lock.json for dependency resolution The root cause was that the typecheck step depends on build tasks (per turbo.json) but wasn't receiving the lockfile check flag, causing skeleton builds to fail. This has been a source of CI instability since the monorepo migration.
… USDC currency (#3090) * Fix Money component compatibility with Customer Account API USDC currency The 2025-07 API update introduced USDC currency code to Customer Account API but not Storefront API, causing TypeScript errors and runtime failures. Changes: - Update Money component to accept MoneyV2 from both Storefront and Customer Account APIs via union types - Enhance useMoney hook to detect unsupported currency codes (like USDC) and gracefully fall back to decimal formatting - Add currency code suffix for unsupported currencies (e.g., "100.00 USDC") to maintain clarity - Default to 2 decimal places for USDC to reinforce its 1:1 USD peg based on industry standards Technical details: - Handle Intl.NumberFormat RangeError for cryptocurrency codes not in ISO 4217 - Add comprehensive test coverage for both API types and USDC formatting - Update TypeScript types to support both CurrencyCode enums Fixes #3089 * Add changeset for Money component USDC compatibility fix * Add build script to copy customer-account-api-types to dist Since our Money component now imports from customer-account-api-types, we need to ensure this file is copied to the dist folder during the build process. Previously, only storefront-api-types was being copied. - Replace copy-storefront-types with copy-api-types script that copies both API type files - Update build and dev:demo scripts to use the new copy-api-types script - This fixes the CI build failure where customer-account-api-types.d.ts was not found in dist * Trigger CI rebuild
During the build process, tsup creates temporary bundled configuration files (tsup.config.bundled_*.mjs) in each package directory. These are generated artifacts that should not be tracked in version control. These files: - Are created when running 'npm run build:all' or similar build commands - Contain bundled versions of tsup.config.ts files for build execution - Have random hash suffixes (e.g., tsup.config.bundled_jt705ss6fz.mjs) - Should be cleaned up automatically but sometimes persist Adding the pattern '**/tsup.config.bundled_*.mjs' to gitignore prevents these temporary build artifacts from cluttering git status and being accidentally committed.
Added @types/inquirer and @types/yargs to cookbook devDependencies to resolve TypeScript compilation errors when running cookbook commands. This fixes the TS7016 error: 'Could not find a declaration file for module inquirer' that occurs when running cookbook commands like 'npm run cookbook -- regenerate' on fresh branches or after clean installs.
Regenerated all cookbook recipes to update patch file hashes after recent changes to the skeleton template. This ensures the cookbook validation passes in CI by keeping the patches in sync with the current state of the skeleton template. The regeneration updates: - All patch files with new content hashes - Recipe YAML files with updated patch references - README and prompt files with current examples This fixes the 'Validate Recipes' CI job failure.
The cookbook validation was failing because the skeleton template intentionally doesn't include a lockfile (templates shouldn't have lockfiles), but the hydrogen build command validates lockfile presence by default. This fix adds SHOPIFY_HYDROGEN_FLAG_LOCKFILE_CHECK=false to the cookbook validation step, consistent with how other CI build tasks handle this requirement.
The @shopify/cli-hydrogen dependency in the root package.json was causing npm to incorrectly attempt to run scripts in all workspace packages when executing root-level scripts like 'npm run build:pkg'. This resulted in confusing error messages when npm tried to run 'build:pkg' in packages that don't have that script defined (e.g., hydrogen-react). This dependency was likely added accidentally during the v3_routeConfig changes in March 2025 (commit 74ef1ba) but serves no actual purpose: - The root package doesn't use cli-hydrogen directly - Turbo.json already manages build dependencies correctly - Workspace packages are linked automatically by npm workspaces - The skeleton template correctly uses @shopify/cli (which bundles cli-hydrogen internally) Removing this dependency: - Eliminates the 'Missing script: build:pkg' error messages - Prevents npm from unnecessarily propagating script execution - Maintains all existing functionality (builds still work correctly) - Simplifies the dependency tree The actual build process through Turbo remains unchanged and continues to work as expected.
* Support imperial units in UnitPriceMeasurement * remove unnecessary changeset * Use count instead of dimension, which doesn't exist * Don't commit claude settings in this PR to avoid cluttering the branch
* Fix React Context errors in Vite dev server This fixes "Cannot read properties of null (reading 'useContext')" errors that occur when React Context hooks are used in development mode. Root Cause: Vite's module federation can cause multiple React instances to be loaded when @Shopify/hydrogen is served as an external module. React Context requires a single React instance to function properly - when multiple instances exist, Context providers from one instance cannot be consumed by hooks from another instance, resulting in null context values. Solution: 1. Added resolve.dedupe for react, react-dom, and @Shopify/hydrogen to ensure Vite uses a single instance of these packages 2. Modified optimizeDeps.include to conditionally optimize @Shopify/hydrogen: - In production/regular projects: Optimize @Shopify/hydrogen to prevent context errors - In monorepo development: Skip optimization to allow live reload of source changes This maintains development workflow in the monorepo while fixing the production build issue. * Fix React Context error during client-side hydration Resolves 'Cannot read properties of null (reading useContext)' error that occurred on first page load when using CSP with nonces. The issue was caused by NonceProvider being present during SSR but missing during client hydration, creating a React Context mismatch. Changes: - Export NonceProvider from @Shopify/hydrogen for client-side usage - Add NonceProvider wrapper to skeleton's entry.client.tsx - Simplify Vite config to improve React Context stability
The Hydrogen cart handler's return type includes a union of three possible user error types: CartUserError, MetafieldsSetUserError, and MetafieldDeleteUserError. While the skeleton template currently only handles non-metafield operations, the TypeScript types from the cart handler don't narrow based on the specific operation performed. This change updates the CartUserErrors, CartMain, and CartSummary components to accept all three error types, resolving TypeScript errors and ensuring the components are compatible with any cart operation that might be added in the future.
* Update Vitest from v1.0.4 to v3.2.4 across all packages - Upgrade vitest to ^3.2.4 in all 5 packages that use it - Upgrade @vitest/coverage-v8 to ^3.2.4 to match vitest version - Add vitest as devDependency to create-hydrogen package (was missing) - Add explicit coverage provider configuration (required in v3+) - Set provider: 'v8' in cli and hydrogen-react vitest configs Breaking change: Vitest v2+ requires explicit coverage provider config * Fix Vitest v3 Mock type breaking changes In Vitest v3, vi.fn() returns Mock<() => unknown> instead of Mock<any[], any>, requiring explicit type assertions or type parameters for mocks with specific return types. Changes: - Add type assertion for fetchWithServerCache mock in storefront.test.ts - Simplify Promise creation in session mocks (Promise.resolve instead of new Promise) - Fix Mock type usage in Image.test.tsx for console.warn assertions - Add vitest/globals to tsconfig for packages using global test functions - Keep jest types alongside vitest types for @testing-library/jest-dom compatibility The stricter typing in v3 catches more potential issues at compile time but requires updating existing mocks that relied on v1's permissive typing. * Fix formatting
…ustomer-account-push flag (#3123) * Fix MiniOxygen to support React Router's redirectDocument for external redirects The Miniflare v4 update introduced a breaking change where dispatchFetch automatically follows redirects internally, preventing external OAuth redirects from working properly. React Router's redirectDocument() expects the browser to handle external redirects, but Miniflare was intercepting and following them within the worker runtime. This fix adds {redirect: 'manual'} to all dispatchFetch calls in MiniOxygen, which prevents Miniflare from following redirects automatically. This ensures that: - External redirects (like OAuth flows to Shopify's Customer Account API) are passed to the browser as intended - React Router's redirectDocument() works correctly for cross-origin navigation - The X-Remix-Reload-Document header is respected for full document reloads The implementation uses Function.prototype.call to pass both arguments to dispatchFetch while maintaining TypeScript compatibility, as the property accessor doesn't properly expose the second parameter in TypeScript definitions. Also adds .tryhydrogen.dev to Vite's allowedHosts for development testing. References: - cloudflare/workers-sdk#5018 - cloudflare/workers-sdk#5191 * Stabilize customer-account-push flag - Changed from customer-account-push__unstable to customer-account-push - Removed hidden: true to make flag visible in help - Fixed typo: Oauth → OAuth in description - Flag now works via CLI argument or SHOPIFY_HYDROGEN_FLAG_CUSTOMER_ACCOUNT_PUSH env var * Fix MiniOxygen redirect handling for OAuth flows Apply {redirect: 'manual'} to Miniflare's dispatchFetch calls to prevent automatic redirect following. This ensures external redirects (like OAuth) are passed to the browser instead of being followed internally. Changes: - Add redirect: 'manual' parameter to dispatchFetch in both worker and node environments - Fix Headers.getSetCookie() usage for proper Set-Cookie header handling - Add comprehensive tests for redirect behavior in both environments - Stabilize customer-account-push CLI flag (remove __unstable prefix) This fixes OAuth/PKCE authentication flows in React Router 7 where redirectDocument() and external redirects need to be handled by the browser, not the server. * Add changeset for OAuth redirect fixes and stabilized CLI flag * Fix intermittent CI test failures in MiniOxygen tests - Use dynamic port allocation (port: 0) for all test servers to prevent EADDRINUSE errors - Properly handle OS-assigned ports when using port 0 - Add server tracking and cleanup to ensure all servers are properly closed - Fix proxy server test to use dynamic port allocation This resolves race conditions and port conflicts that occur when tests run in parallel in CI environments. * Fix TypeScript errors in MiniOxygen tests - Change MiniOxygenOptions to MiniOxygenPreviewOptions in test files - Add proper type imports for Request, Response, and DispatchFetch - Add explicit types for onRequest callback parameters This resolves TypeScript compilation errors that were failing CI.
Oxygen deployed a preview of your
Learn more about Hydrogen's GitHub integration. |
b5fe6a4
to
49eea20
Compare
2912d7e
to
ac77705
Compare
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.
WHY are these changes introduced?
Adds more safeguards + helpful information to CLAUDE.md to help it help us
HOW to test your changes?
Post-merge steps
Checklist