Skip to content

Fix: normalize cached store URL before comparison in storeContext#7019

Open
isaacroldan wants to merge 1 commit intomainfrom
fix/normalize-cached-store-url-comparison
Open

Fix: normalize cached store URL before comparison in storeContext#7019
isaacroldan wants to merge 1 commit intomainfrom
fix/normalize-cached-store-url-comparison

Conversation

@isaacroldan
Copy link
Contributor

The cached dev_store_url from app.toml or hidden config was compared against the already-normalized selectedStore.shopDomain without first normalizing it. This meant format differences (e.g. 'https://store.myshopify.com' vs 'store.myshopify.com', or 'store' vs 'store.myshopify.com') would cause a false mismatch and trigger unnecessary writes to .shopify/project.json on every dev run.

Now we normalize cachedStoreURL before comparing, so the hidden config is only updated when the store actually changes.

WHY are these changes introduced?

Fixes #0000

WHAT is this pull request doing?

How to test your changes?

Post-release steps

Measuring impact

How do we know this change was effective? Please choose one:

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
  • Existing analytics will cater for this addition
  • PR includes analytics changes to measure impact

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes

The cached dev_store_url from app.toml or hidden config was compared
against the already-normalized selectedStore.shopDomain without first
normalizing it. This meant format differences (e.g. 'https://store.myshopify.com'
vs 'store.myshopify.com', or 'store' vs 'store.myshopify.com') would cause
a false mismatch and trigger unnecessary writes to .shopify/project.json
on every `dev` run.

Now we normalize cachedStoreURL before comparing, so the hidden config
is only updated when the store actually changes.
@isaacroldan isaacroldan requested a review from a team as a code owner March 16, 2026 17:05
Copilot AI review requested due to automatic review settings March 16, 2026 17:05
@github-actions
Copy link
Contributor

We detected some changes at packages/*/src and there are no updates in the .changeset.
If the changes are user-facing, run pnpm changeset add to track your changes and include them in the next release CHANGELOG.

Caution

DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes repeated hidden-config updates during dev runs by normalizing the cached dev_store_url (from app.toml or hidden config) before comparing it to the normalized selectedStore.shopDomain, avoiding false mismatches due to formatting differences.

Changes:

  • Normalize cachedStoreURL via normalizeStoreFqdn before comparing to the selected store.
  • Add test coverage for format-only differences (protocol prefix, short store name) and for actual store changes.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
packages/app/src/cli/services/store-context.ts Normalizes cached store URL before comparison to prevent unnecessary hidden-config writes.
packages/app/src/cli/services/store-context.test.ts Adds tests ensuring hidden config is not updated for format-only differences and is updated when the store changes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +234 to +238
const hiddenConfig = await readFile(appHiddenConfigPath(dir))
// The file was initialized empty, so if updateHiddenConfig was NOT called,
// it should still be empty
expect(hiddenConfig).toEqual('')
})
Comment on lines +263 to +269
vi.mocked(fetchStore).mockResolvedValue(storeMatchingNormalized)

await storeContext({appContextResult: updatedAppContextResult, forceReselectStore: false})

// The hidden config should NOT be updated since normalized URLs match
const hiddenConfig = await readFile(appHiddenConfigPath(dir))
expect(hiddenConfig).toEqual('')
@github-actions
Copy link
Contributor

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements 77.31% 14587/18869
🟡 Branches 70.92% 7240/10209
🟡 Functions 76.3% 3706/4857
🟡 Lines 78.8% 13791/17502

Test suite run success

3814 tests passing in 1473 suites.

Report generated by 🧪jest coverage report action from 71274f0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants