diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-alias-chain.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-alias-chain.ts new file mode 100644 index 00000000000..b3d0be65800 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-alias-chain.ts @@ -0,0 +1,6 @@ +// Alias CHAIN: const a = shopify; const b = a. Complex resolves through the +// chain; simple can't read the event but WARNS (never a silent miss). +declare const shopify: {intercept: (event: string, handler: () => void) => void} +const a = shopify +const b = a +b.intercept('aliaschain', () => {}) diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-alias-noise.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-alias-noise.ts new file mode 100644 index 00000000000..d070c98f2de --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-alias-noise.ts @@ -0,0 +1,5 @@ +// A shopify-object alias used for NON-intercept reasons. The simple detector +// must NOT warn here — it only flags aliases when `.intercept` is accessed. +declare const shopify: {intercept: (e: string, h: () => void) => void; toast: {show: (m: string) => void}} +const s = shopify +s.toast.show('hi') diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-branches.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-branches.ts new file mode 100644 index 00000000000..4ccbebadbc6 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-branches.ts @@ -0,0 +1,6 @@ +declare const shopify: {intercept: (event: string, handler: () => void) => void} +if (Math.random() > 0.5) { + shopify.intercept('branchif', () => {}) +} else { + shopify.intercept('branchelse', () => {}) +} diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-crossfile-dep.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-crossfile-dep.ts new file mode 100644 index 00000000000..4bd8aa908e0 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-crossfile-dep.ts @@ -0,0 +1,2 @@ +declare const shopify: {intercept: (event: string, handler: () => void) => void} +export const block = shopify.intercept diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-crossfile.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-crossfile.ts new file mode 100644 index 00000000000..87ece2760b8 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-crossfile.ts @@ -0,0 +1,2 @@ +import {block} from './case-crossfile-dep.js' +block('crossfile', () => {}) diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-destructure.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-destructure.ts new file mode 100644 index 00000000000..72a40ddd307 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-destructure.ts @@ -0,0 +1,3 @@ +declare const shopify: {intercept: (event: string, handler: () => void) => void} +const {intercept} = shopify +intercept('destructured', () => {}) diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-direct.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-direct.ts new file mode 100644 index 00000000000..67e35b7d4d2 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-direct.ts @@ -0,0 +1,2 @@ +declare const shopify: {intercept: (event: string, handler: () => void) => void} +shopify.intercept('beforecheckout', () => {}) diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-dynamic.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-dynamic.ts new file mode 100644 index 00000000000..9409f849dff --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-dynamic.ts @@ -0,0 +1,3 @@ +declare const shopify: {intercept: (event: string, handler: () => void) => void} +const evt = 'dynamic' +shopify.intercept(evt, () => {}) diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-missing-callback.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-missing-callback.ts new file mode 100644 index 00000000000..51a356e23f3 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-missing-callback.ts @@ -0,0 +1,5 @@ +// Literal event but NO callback second arg. The real API requires a callback, +// so this is a suspected MALFORMED registration: surfaced (complex: unresolved, +// simple: missing-callback warning), never silently counted as an event. +declare const shopify: {intercept: (event: string, handler?: () => void) => void} +shopify.intercept('missingcb') diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-object-alias.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-object-alias.ts new file mode 100644 index 00000000000..d539e2207b0 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-object-alias.ts @@ -0,0 +1,3 @@ +declare const shopify: {intercept: (event: string, handler: () => void) => void} +const s = shopify +s.intercept('objectalias', () => {}) diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-reassign.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-reassign.ts new file mode 100644 index 00000000000..7af18fcd7d5 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-reassign.ts @@ -0,0 +1,4 @@ +declare const shopify: {intercept: (event: string, handler: () => void) => void} +let fn: typeof shopify.intercept +fn = shopify.intercept +fn('reassigned', () => {}) diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-trailing-args.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-trailing-args.ts new file mode 100644 index 00000000000..067520fcb29 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-trailing-args.ts @@ -0,0 +1,4 @@ +// Extra trailing args beyond ('event', callback) are tolerated — the event +// still resolves from the first arg with a callback present. +declare const shopify: {intercept: (event: string, handler: () => void, ...rest: unknown[]) => void} +shopify.intercept('trailing', () => {}, 'extra') diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-demo/src/aliased.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-demo/src/aliased.ts new file mode 100644 index 00000000000..6b5bb012552 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-demo/src/aliased.ts @@ -0,0 +1,5 @@ +// The intercept reference is aliased AND re-exported from here. The detector +// must resolve `block` (imported elsewhere) back to shopify.intercept. +declare const shopify: {intercept: (event: string, handler: () => void) => void} + +export const block = shopify.intercept diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-demo/src/checkout.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-demo/src/checkout.ts new file mode 100644 index 00000000000..be29ddc4b76 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-demo/src/checkout.ts @@ -0,0 +1,7 @@ +// Imported file — its callsites are part of the extension's capability surface. +export function registerCheckoutGuards() { + const {intercept} = shopify + intercept('beforecapture', () => {}) +} + +declare const shopify: {intercept: (event: string, handler: () => void) => void} diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-demo/src/home-tile.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-demo/src/home-tile.ts new file mode 100644 index 00000000000..2b726a460ca --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-demo/src/home-tile.ts @@ -0,0 +1,8 @@ +// Module for the `pos.home.tile.render` target. This target does NOT support +// intercepts, so the detector must NOT scan it. If scoping is broken, the event +// below would leak into the derived capabilities. +declare const shopify: {intercept: (event: string, handler: () => void) => void} + +export function renderTile() { + shopify.intercept('shouldnotappear', () => {}) +} diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-demo/src/index.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-demo/src/index.ts new file mode 100644 index 00000000000..d0da03d237b --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-demo/src/index.ts @@ -0,0 +1,47 @@ +// Demo POS UI extension entry. Exercises every alias path the detector must +// handle. The `shopify` global is injected by the host at runtime. +declare const shopify: {intercept: (event: string, handler: () => void) => void} + +import {registerCheckoutGuards} from './checkout.js' +import {block} from './aliased.js' + +// 1. Direct call. +shopify.intercept('beforecheckout', () => {}) + +// 2. Aliased via `const fn = shopify.intercept` (declared before use). +const guard = shopify.intercept +guard('beforepayment', () => {}) + +// 3. Destructured: `const {intercept} = shopify`. +const {intercept} = shopify +intercept('beforediscount', () => {}) + +// 4. Destructured + renamed: `const {intercept: rename} = shopify`. +const {intercept: renamed} = shopify +renamed('beforerefund', () => {}) + +// 5. shopify object aliased, then member call: `const s = shopify; s.intercept(...)`. +const s = shopify +s.intercept('beforeexchange', () => {}) + +// 6. Reassignment of a let binding. +let later +later = shopify.intercept +later('beforevoid', () => {}) + +// 7. Cross-file re-exported intercept reference. +block('beforecancel', () => {}) + +// 8. Inside control flow — MUST still be detected (reachability ignored). +if (Math.random() > 0.5) { + shopify.intercept('beforetax', () => {}) +} else { + shopify.intercept('beforeshipping', () => {}) +} + +// 9. UNRESOLVED: dynamic/computed event args — surfaced, never dropped. +const dynamicEvent = 'beforesomething' +shopify.intercept(dynamicEvent, () => {}) +shopify.intercept(`before${'checkout'}`, () => {}) + +registerCheckoutGuards() diff --git a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.DESIGN.md b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.DESIGN.md new file mode 100644 index 00000000000..132b486e20b --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.DESIGN.md @@ -0,0 +1,265 @@ +# SPIKE: Deriving POS intercept events from source code + +Branch: `henry/pos-intercept-ast-derive` (exploration only) + +## What this prototype does + +Instead of trusting the hand-authored `capabilities.intercepts` TOML array, it +**derives** the intercept events from the extension's **source code** via a +control-flow-agnostic AST walk of the import graph. + +Files: + +- `pos_intercept_detection.ts` — the full detector (`detectPosIntercepts`, + `deriveInterceptsFromConfig`, `findInterceptEntryModules`, `POS_INTERCEPT_TARGET`). +- `pos_intercept_detection_simple.ts` — a deliberately simple baseline detector + (`detectPosInterceptsSimple`) for evaluating whether the complexity is worth it. +- `pos_intercept_detection.test.ts` — unit tests against the demo fixture. +- `pos_intercept_detection_comparison.test.ts` — complex-vs-simple matrix. +- `fixtures/pos-intercept-demo/` — a POS extension exercising every alias form, + control-flow branches, a cross-file re-exported alias, and dynamic args, plus a + render-target module (`home-tile.ts`) that must be IGNORED. +- `fixtures/pos-intercept-compare/` — one isolated file per pattern for the matrix. +- `pos_ui_extension.ts` — `deployConfig` now calls `deriveAndMergeIntercepts`. +- `pos_ui_extension.test.ts` — proves the derived events land in the deployed + `capabilities.intercepts`. + +## Entry points come from declared TARGETS, scoped to `pos.app.ready.data` + +Entry points are NOT guessed filenames (`index.*`). They are the `module` paths +of the extension's declared targets (`targeting` / legacy `extension_points`, +per `NewExtensionPointSchema`). Detection is scoped to a single target: + +> **`pos.app.ready.data`** — the session-lifetime BACKGROUND target. + +Confirmed against the published `@shopify/ui-extensions` point-of-sale surface: +`.../surfaces/point-of-sale/targets/pos.app.ready.data.d.ts` re-exports +`BackgroundShopifyGlobal as ShopifyGlobal`, and `globals.d.ts` documents that the +background-only global (valid only from `pos.app.ready.data`) is what carries +the host-mediated event/intercept APIs. Render targets +(`pos.home.tile.render`, `pos.home.modal.render`, …) see the narrower +`ShopifyGlobal` and do not support intercepts, so we don't scan them. +`findInterceptEntryModules(config, directory)` filters targets to +`pos.app.ready.data` and returns their resolved `module` paths as the entry set. + +### Detection behaviour (matches the brief) + +1. **Reuses the CLI's existing AST tooling.** It calls `findAllImportedFiles()` + from `type-generation.ts` to walk the full import graph from the + `pos.app.ready.data` target's `module`(s), and uses the same `typescript` + compiler API (`ts.createSourceFile` / `forEachChild` / `isCallExpression`). +2. **Ignores control flow.** Every `shopify.intercept(...)` callsite is counted + regardless of `if`/`else`/ternary/loop/dead-code. Capabilities are a static + **superset** of what the extension might do; reachability is a runtime + concern we deliberately do not evaluate. +3. **Tracks the intercept function value through aliasing.** A whole-graph + fixpoint resolves the following back to `shopify.intercept`: + - `const guard = shopify.intercept` + - `const {intercept} = shopify` + - `const {intercept: renamed} = shopify` + - `const s = shopify; s.intercept('...')` (object-alias then member access) + - `let fn; fn = shopify.intercept` (reassignment) + - `export const block = shopify.intercept` imported/called in another file + (cross-file re-export) +4. **Only string-literal first args are resolvable.** Variables, member + expressions and template strings with substitutions are surfaced as + `unresolved` with file/line and the raw arg text — never silently dropped. + +Run the tests: + +``` +cd packages/app +../../node_modules/.bin/vitest run \ + src/cli/models/extensions/specifications/pos_intercept_detection.test.ts \ + src/cli/models/extensions/specifications/pos_ui_extension.test.ts \ + src/cli/models/extensions/specifications/pos_intercept_detection_comparison.test.ts +``` + +## Complex (resolve) vs "safe simplest" (resolve-or-warn) + +The second detector (`pos_intercept_detection_simple.ts`) is now the **safe +simplest** design. Its rule: **never silently under-report.** A call is either +DERIVED SILENTLY (direct `shopify.intercept('')`) or FLAGGED +LOUDLY (a warning with file:line + raw source telling the developer to declare +that intercept in the TOML). It resolves nothing indirect and follows nothing +cross-file — it recognizes intercept-shaped patterns *syntactically* and warns. + +All calls use the real API shape `shopify.intercept('', callback)`. + +Three-way matrix (`pos_intercept_detection_comparison.test.ts`): + +| pattern | complex resolves | simple resolves | simple warns | +|---------|:-------:|:-------:|:-----| +| `shopify.intercept('beforecheckout', cb)` | yes | yes | — | +| `const {intercept} = shopify; intercept('x', cb)` | yes | no | `destructure` | +| `const s = shopify; s.intercept('x', cb)` | yes | no | `object-alias-access` | +| `let fn; fn = shopify.intercept; fn('x', cb)` | yes | no | `function-reference` | +| cross-file `export const block = shopify.intercept` | yes | no | `function-reference` (at creation site) | +| `if/else` both branches | yes | yes | — | +| dynamic variable first arg | unresolved | no | `dynamic-arg` | +| literal event, NO callback (malformed) | unresolved | no | `missing-callback` | +| `('event', cb, extraArg)` trailing args | yes | yes | — | +| `const s = shopify; s.toast.show()` (noise) | n/a | no | **— (no false positive)** | + +**Key property:** the simple detector has ZERO silent misses on the alias +patterns — where it can't resolve, it warns. + +### Callback-presence handling (2-arg API `intercept('', callback)`) + +The event is the FIRST arg; a genuine registration has a callback SECOND arg. +Both detectors accept as a callback: an arrow function, a function expression, +or a function reference (identifier or property access). Handling: + +- **Literal event + callback** → resolved event (counted). This is the only path + that counts an event. +- **Literal event, NO/invalid callback** → surfaced but NOT counted: a suspected + MALFORMED registration. Complex reports it in `unresolved`; simple emits a + `missing-callback` warning. (Henry's lean: surface, don't silently count.) +- **Non-literal first arg** → `dynamic-arg` unresolved/warn as before (callback + presence is moot — the event name is unknowable). +- **Extra trailing args** beyond `(event, callback)` → tolerated; only `args[0]` + and `args[1]` are inspected. + +### Is warn-on-alias simpler or more complex than full resolution? + +**Honest assessment: simpler, and meaningfully so.** Code-ish (non-comment) +lines: complex ≈ 295, simple ≈ 172 (~42% smaller). More importantly, the simple +detector DROPS the expensive machinery: + +- No cross-file alias propagation and no whole-graph fixpoint loop. +- No per-file import/export symbol maps. +- No multi-pass alias-set growth (`shopifyAliases`/`interceptAliases`). +- No `let`-reassignment data-flow. + +What it ADDS is cheap and local: a single-pass syntactic check per file that (a) +resolves direct literal calls and (b) pattern-matches four warn shapes. The only +state it keeps is a one-level set of `const s = shopify` alias names, used *only* +to scope the object-alias warning. + +### False-positive / noise risk and how it's scoped + +- **`const s = shopify` used for non-intercept reasons.** We do NOT warn on the + alias declaration. We warn only at an actual `.intercept` access on that alias + (`s.intercept`). A `const s = shopify; s.toast.show()` produces no warning + (covered by the `case-alias-noise` fixture). +- **`function-reference` breadth.** Any non-call use of `shopify.intercept` + warns. This is intentional and low-noise — there is essentially no legitimate + reason to reference `shopify.intercept` without eventually calling it. +- **Residual noise vector:** a developer who aliases the shopify object *and* + legitimately calls `.intercept` with a literal (`const s = shopify; + s.intercept('beforecheckout')`) gets a warning even though the event is + knowable. That's the deliberate trade: the simple detector refuses to resolve + through the alias, so it asks for an explicit declaration instead. The complex + detector resolves it silently. This is exactly the cost/benefit Henry is + weighing. +- **Single-level aliasing.** `const a = shopify; const b = a; b.intercept()` is + not recognized as an alias chain, so `b.intercept` would warn as a + `function-reference`/miss depending on shape — still a warning, never a silent + miss, so the safety property holds. + +--- + +## THE KEY DESIGN QUESTION: derived events → backend + +> If events are derived and therefore **not** present in the TOML, how does that +> information get stored in / transmitted to the backend? + +### Answer: reuse the existing `capabilities.intercepts` wire field. No new mechanism. + +There is **no dedicated transport for "derived" events** and there does not need +to be one. The transmission mechanism is the deployed **version config**, which +the CLI already builds per module in `deployConfig`. + +Concrete trace of how config reaches the backend on deploy: + +``` +pos_ui_extension.deployConfig(config, directory) + → returns { name, description, renderer_version, capabilities } + capabilities.intercepts ← derived ∪ TOML-declared events +ExtensionInstance.deployConfig() (extension-instance.ts:198) +ExtensionInstance.bundleConfig() (extension-instance.ts:402) + → { config: JSON.stringify(configValue), context, handle, uid, uuid, ... } +services/deploy.ts:254 app.allExtensions.map(ext.bundleConfig(...)) + → uploadExtensionsBundle({ appModules, ... }) + → backend persists each module's `config` blob as the version config +``` + +So the derived events ride inside the module's serialized `config` string, +**byte-for-byte identical** to how a TOML-declared `capabilities.intercepts` +array would be serialized. The backend consumer that reads +`capabilities.intercepts` today keeps working unchanged — it cannot tell (and +does not care) whether the array came from the TOML or from AST derivation. + +This is exactly what the prototype does: + +```ts +// pos_ui_extension.ts deployConfig +const capabilities = await deriveAndMergeIntercepts(config.capabilities, directory) +return { name, description, renderer_version, capabilities } +``` + +`deriveAndMergeIntercepts` **unions** derived events with any TOML-declared ones, +de-dupes, and sorts. That gives us three coexisting modes with zero backend +change: + +| Mode | TOML `intercepts` | Source | Emitted to backend | +|------|-------------------|--------|--------------------| +| Derivation-first (goal) | absent | `intercept('beforecheckout')` | `['beforecheckout']` | +| Declaration-only (today) | `['beforecheckout']` | none/unparseable | `['beforecheckout']` | +| Hybrid | `['manuallydeclared']` | `intercept('beforecheckout')` | `['beforecheckout','manuallydeclared']` | + +### Why this is the right shape (per the 2022 "source of truth" ADR) + +The ADR's principle — *"we derive the information if it can be derived"* — is +satisfied without changing the contract the backend already depends on. The TOML +field becomes an **optional override / escape hatch** rather than the required +source of truth. The deployed version config remains the single artifact of +record; derivation just changes *who fills in* `capabilities.intercepts` +(the compiler vs. the author), not *where it is stored*. + +### Alternatives considered (and rejected for this spike) + +- **New top-level `derived_intercepts` wire field.** Rejected: forces a backend + schema change and a dual-read code path for no benefit — the semantics are + identical to `capabilities.intercepts`. +- **Emit from the esbuild metafile instead of a standalone AST pass.** + `bundle.ts` already runs esbuild with `metafile: true` on production deploy. + The metafile gives us the exact module graph esbuild bundles (more accurate + than our hand-rolled resolver), but it does **not** contain callsite/AST + detail, so we'd still need an AST pass over those files. A follow-up could feed + the metafile's input list into `detectPosIntercepts` instead of + `findAllImportedFiles` for graph fidelity. Out of scope for the spike. + +--- + +## Reliability gaps (must be surfaced, not hidden) + +1. **Dynamic / computed event args are unresolvable.** `intercept(evt)`, + `intercept(obj.event)`, `` intercept(`before${x}`) `` cannot be statically + resolved. The detector reports them in `result.unresolved` (with file:line + + raw text) and `deriveAndMergeIntercepts` logs an `outputWarn`. **Product + decision needed:** warn-and-continue (current), hard-fail the deploy, or + require these to be TOML-declared. The TOML override path exists precisely for + this case. +2. **Import-graph resolution is best-effort.** The cross-file resolver handles + relative imports and the TS `./x.js`→`x.ts` convention, and skips + `node_modules`. `tsconfig` path aliases (`@/...`) are handled by the reused + `findAllImportedFiles` for graph walking but **not** by the cross-file + *alias-propagation* resolver — an intercept reference re-exported through a + path-aliased module could be missed. Unifying both on `ts.resolveModuleName` + would close this. +3. **Aliasing is intentionally shallow (basic data-flow).** We resolve direct + assignments, destructuring, object-aliasing and cross-file re-exports. We do + **not** track the reference through: function parameters / higher-order + passing (`wrap(shopify.intercept)`), array/object property storage + (`const m = {i: shopify.intercept}; m.i(...)`), or `.bind()/.call()`. These + would silently under-report. A full type-checker-backed symbol analysis + (`ts.createProgram` + `TypeChecker`) would be more robust than the + `createSourceFile`-per-file approach but is heavier. +4. **Dead code is included by design.** Because control flow is ignored, an + intercept in unreachable code still becomes a capability. This is the safe + direction (over-declaration) but means derived capabilities can be broader + than runtime behaviour. +5. **No JS-runtime evaluation.** Constant folding (`const E = 'beforecheckout'; + intercept(E)`) is not performed; `E` is reported unresolved even though it is + a compile-time constant. Simple const-propagation is a cheap future win. diff --git a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.test.ts b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.test.ts new file mode 100644 index 00000000000..49ee45d839f --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.test.ts @@ -0,0 +1,102 @@ +import { + detectPosIntercepts, + deriveInterceptsFromConfig, + findInterceptEntryModules, + POS_INTERCEPT_TARGET, +} from './pos_intercept_detection.js' +import {joinPath} from '@shopify/cli-kit/node/path' +import {fileURLToPath} from 'node:url' +import {dirname} from 'node:path' +import {describe, expect, test} from 'vitest' + +const currentDir = dirname(fileURLToPath(import.meta.url)) +const fixtureDir = joinPath(currentDir, 'fixtures', 'pos-intercept-demo') + +// A parsed config for the demo extension: the intercept-supporting background +// target points at src/index.ts; a render target (which must be IGNORED) points +// at src/home-tile.ts. +const demoConfig = { + targeting: [ + {target: POS_INTERCEPT_TARGET, module: 'src/index.ts'}, + {target: 'pos.home.tile.render', module: 'src/home-tile.ts'}, + ], +} + +describe('findInterceptEntryModules', () => { + test('returns only the pos.app.ready.data target module, ignoring render targets', () => { + const modules = findInterceptEntryModules(demoConfig, fixtureDir) + expect(modules).toEqual([joinPath(fixtureDir, 'src/index.ts')]) + }) + + test('reads the legacy extension_points field too', () => { + const modules = findInterceptEntryModules( + {extension_points: [{target: POS_INTERCEPT_TARGET, module: 'src/index.ts'}]}, + fixtureDir, + ) + expect(modules).toEqual([joinPath(fixtureDir, 'src/index.ts')]) + }) + + test('returns nothing when no intercept-supporting target is declared', () => { + expect(findInterceptEntryModules({targeting: [{target: 'pos.home.tile.render', module: 'src/home-tile.ts'}]}, fixtureDir)).toEqual([]) + }) +}) + +describe('detectPosIntercepts', () => { + test('derives events across the import graph, ignoring control flow and tracking aliases', async () => { + // Given the intercept target module as the entry. + const [entry] = findInterceptEntryModules(demoConfig, fixtureDir) + + // When + const result = await detectPosIntercepts(entry!) + + // Then — every resolvable event, regardless of alias form or control flow. + // 'shouldnotappear' lives in the render-target module and must NOT appear. + expect(result.events).toEqual([ + 'beforecancel', // cross-file re-exported alias + 'beforecapture', // imported file destructured alias + 'beforecheckout', // direct call + 'beforediscount', // const {intercept} = shopify + 'beforeexchange', // const s = shopify; s.intercept(...) + 'beforepayment', // const guard = shopify.intercept + 'beforerefund', // const {intercept: renamed} = shopify + 'beforeshipping', // else branch — reachability ignored + 'beforetax', // if branch — reachability ignored + 'beforevoid', // let reassignment + ]) + expect(result.events).not.toContain('shouldnotappear') + }) + + test('surfaces dynamic/computed event args as unresolved instead of dropping them', async () => { + const [entry] = findInterceptEntryModules(demoConfig, fixtureDir) + const result = await detectPosIntercepts(entry!) + + // Two dynamic callsites: a variable and a template string with substitution. + expect(result.unresolved).toHaveLength(2) + expect(result.unresolved.every((callsite) => callsite.event === null)).toBe(true) + expect(result.unresolved.map((callsite) => callsite.argText).sort()).toEqual([ + '`before${\'checkout\'}`', + 'dynamicEvent', + ]) + result.unresolved.forEach((callsite) => { + expect(callsite.unresolvedReason).toBeTruthy() + expect(callsite.line).toBeGreaterThan(0) + }) + }) +}) + +describe('deriveInterceptsFromConfig', () => { + test('drives detection from the parsed config + directory (target-scoped)', async () => { + const result = await deriveInterceptsFromConfig(demoConfig, fixtureDir) + expect(result?.events).toContain('beforecheckout') + expect(result?.events).not.toContain('shouldnotappear') + expect(result?.analyzedFiles.length).toBeGreaterThanOrEqual(3) + }) + + test('returns undefined when no intercept target is declared', async () => { + const result = await deriveInterceptsFromConfig( + {targeting: [{target: 'pos.home.tile.render', module: 'src/home-tile.ts'}]}, + fixtureDir, + ) + expect(result).toBeUndefined() + }) +}) diff --git a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.ts b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.ts new file mode 100644 index 00000000000..064f24f114b --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.ts @@ -0,0 +1,482 @@ +import {findAllImportedFiles} from './type-generation.js' +import {fileExists, readFileSync} from '@shopify/cli-kit/node/fs' +import {dirname, joinPath, resolvePath} from '@shopify/cli-kit/node/path' +import {uniq} from '@shopify/cli-kit/common/array' +import type ts from 'typescript' + +// --------------------------------------------------------------------------- +// SPIKE / PROTOTYPE: derive POS intercept events from extension SOURCE CODE. +// +// Instead of trusting the hand-authored `capabilities.intercepts` TOML array, +// this walks the extension's import graph and statically detects every +// `shopify.intercept('', ...)` callsite. +// +// ENTRY POINTS COME FROM THE DECLARED TARGETS, not from guessed filenames. Each +// target/extension-point in the parsed config carries a `module` path (see +// NewExtensionPointSchema). We SCOPE detection to a single target: +// `pos.app.ready.data` — the session-lifetime BACKGROUND target (it exposes the +// `BackgroundShopifyGlobal`, under which `intercept` lives). The render targets +// (pos.home.tile.render, pos.home.modal.render, etc.) do not support intercepts, +// so we don't scan them. We walk the import graph from that target's module(s). +// +// Design notes / deliberate choices (per the spike brief): +// * We IGNORE control flow. Every callsite counts, regardless of whether it +// sits inside an `if`, a ternary, a loop, or dead code. Reachability is a +// runtime concern; capabilities are a static superset of "what this +// extension might do". +// * We TRACK THE INTERCEPT FUNCTION VALUE through aliasing. `shopify` is a +// global binding injected by the host, so any of the following resolve back +// to `shopify.intercept`: +// const intercept = shopify.intercept +// const {intercept} = shopify +// const {intercept: block} = shopify +// const s = shopify; s.intercept('...') +// let fn; fn = shopify.intercept (reassignment) +// export const intercept = shopify.intercept (re-export, cross-file) +// * The API is `shopify.intercept('', callback)`: the event is the +// FIRST arg and a callback is the SECOND. Only STRING-LITERAL first args are +// statically resolvable; anything else (variables, member expressions, +// template strings with substitutions) is surfaced as UNRESOLVED, never +// dropped. A resolved event is counted ONLY when a callback second arg is +// present; a literal-event call missing its callback is surfaced as a +// suspected MALFORMED registration (not counted). Extra trailing args are +// tolerated. +// --------------------------------------------------------------------------- + +async function loadTypeScript(): Promise { + // typescript is CJS; dynamic import wraps it as { default: ... } + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const mod: any = await import('typescript') + return mod.default ?? mod +} + +/** A single detected `shopify.intercept(...)` callsite. */ +export interface InterceptCallsite { + /** Resolved event name, or null when the first arg is not a string literal. */ + event: string | null + file: string + line: number + column: number + /** Source text of the first argument, for diagnostics. */ + argText: string + /** Populated only when `event` is null: why we couldn't resolve it. */ + unresolvedReason?: string +} + +export interface InterceptDetectionResult { + /** Unique, sorted set of statically-resolved event names. */ + events: string[] + /** Every callsite we found (resolved and unresolved). */ + callsites: InterceptCallsite[] + /** Callsites whose event arg could not be statically resolved. */ + unresolved: InterceptCallsite[] + /** Files walked (entry + transitive local imports). */ + analyzedFiles: string[] +} + +const INTERCEPT_PROPERTY = 'intercept' +const SHOPIFY_GLOBAL = 'shopify' + +interface FileAnalysis { + path: string + sourceFile: ts.SourceFile + /** Identifiers in this file that point at the `shopify` global object. */ + shopifyAliases: Set + /** Identifiers in this file that point at the `shopify.intercept` function. */ + interceptAliases: Set + /** exportedName -> localName for `export {x}` / `export const x = ...`. */ + exports: Map + /** localName -> {resolvedPath, importedName} for imports we could resolve. */ + imports: Map +} + +/** + * The ONLY POS target that supports intercepts: the session-lifetime background + * target. Its module is the entry point for intercept derivation. Confirmed + * against the @shopify/ui-extensions point-of-sale surface, where + * `pos.app.ready.data` re-exports `BackgroundShopifyGlobal` (the only global + * that carries the host-mediated event/intercept APIs). + */ +export const POS_INTERCEPT_TARGET = 'pos.app.ready.data' + +/** Minimal shape of a declared target/extension-point (see NewExtensionPointSchema). */ +export interface ExtensionTargetLike { + target?: string + module?: string +} + +/** + * Given an extension's parsed configuration and directory, return the absolute + * module path(s) for the intercept-supporting target (`pos.app.ready.data`). + * + * Targets are read from `targeting` (preferred) or the legacy `extension_points` + * array — exactly the fields ui_extension uses. Entry points are the declared + * `module` paths, NOT a guessed `index.*` filename. + */ +export function findInterceptEntryModules( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + config: {targeting?: any; extension_points?: any} | undefined, + directory: string, +): string[] { + const rawTargets = config?.targeting ?? config?.extension_points ?? [] + if (!Array.isArray(rawTargets)) return [] + + return (rawTargets as ExtensionTargetLike[]) + .filter((entry) => entry?.target === POS_INTERCEPT_TARGET && typeof entry?.module === 'string') + .map((entry) => resolvePath(directory, entry.module as string)) +} + +function scriptKindFor(ts: typeof import('typescript'), filePath: string): ts.ScriptKind { + if (filePath.endsWith('.ts')) return ts.ScriptKind.TS + if (filePath.endsWith('.tsx')) return ts.ScriptKind.TSX + return ts.ScriptKind.JSX +} + +/** Best-effort relative-module resolver (node_modules are intentionally skipped). */ +async function resolveLocalModule(importPath: string, fromFile: string): Promise { + if (!importPath.startsWith('./') && !importPath.startsWith('../')) return undefined + // TS allows importing `./x.js` to refer to `x.ts`; strip a trailing JS-ish + // extension so the candidate list below can re-add the real source extension. + const normalized = importPath.replace(/\.(js|jsx|mjs|cjs)$/, '') + const base = resolvePath(dirname(fromFile), normalized) + const exts = ['', '.ts', '.tsx', '.js', '.jsx'] + for (const ext of exts) { + const withExt = base + ext + // eslint-disable-next-line no-await-in-loop + if (await fileExists(withExt)) return withExt + } + for (const ext of ['.ts', '.tsx', '.js', '.jsx']) { + const indexPath = joinPath(base, `index${ext}`) + // eslint-disable-next-line no-await-in-loop + if (await fileExists(indexPath)) return indexPath + } + return undefined +} + +/** Is `expr` a member access of the intercept property off a shopify alias? */ +function isShopifyInterceptAccess( + ts: typeof import('typescript'), + expr: ts.Expression, + shopifyAliases: Set, +): boolean { + if (ts.isPropertyAccessExpression(expr)) { + return ( + ts.isIdentifier(expr.expression) && + shopifyAliases.has(expr.expression.text) && + expr.name.text === INTERCEPT_PROPERTY + ) + } + if (ts.isElementAccessExpression(expr)) { + return ( + ts.isIdentifier(expr.expression) && + shopifyAliases.has(expr.expression.text) && + ts.isStringLiteralLike(expr.argumentExpression) && + expr.argumentExpression.text === INTERCEPT_PROPERTY + ) + } + return false +} + +/** + * Scan a file's declarations/assignments once, growing the shopify- and + * intercept-alias sets based on their current contents. Returns true if either + * set grew (so callers can run to a fixpoint — aliases may be defined out of + * order or depend on each other). + */ +function growAliases(ts: typeof import('typescript'), analysis: FileAnalysis): boolean { + const before = analysis.shopifyAliases.size + analysis.interceptAliases.size + const {shopifyAliases, interceptAliases} = analysis + + const considerBinding = (name: ts.BindingName, init: ts.Expression | undefined): void => { + if (!init) return + + // Identifier target: const x = + if (ts.isIdentifier(name)) { + if (ts.isIdentifier(init)) { + if (shopifyAliases.has(init.text)) shopifyAliases.add(name.text) + if (interceptAliases.has(init.text)) interceptAliases.add(name.text) + } else if (isShopifyInterceptAccess(ts, init, shopifyAliases)) { + interceptAliases.add(name.text) + } + return + } + + // Object destructuring target: const {intercept} = shopify / shopifyAlias + if (ts.isObjectBindingPattern(name) && ts.isIdentifier(init) && shopifyAliases.has(init.text)) { + for (const element of name.elements) { + const propName = + element.propertyName && ts.isIdentifier(element.propertyName) + ? element.propertyName.text + : ts.isIdentifier(element.name) + ? element.name.text + : undefined + if (propName === INTERCEPT_PROPERTY && ts.isIdentifier(element.name)) { + interceptAliases.add(element.name.text) + } + } + } + } + + const visit = (node: ts.Node): void => { + // const/let/var declarations + if (ts.isVariableDeclaration(node)) { + considerBinding(node.name, node.initializer) + } + + // Reassignment: fn = shopify.intercept ; s = shopify + if (ts.isBinaryExpression(node) && node.operatorToken.kind === ts.SyntaxKind.EqualsToken) { + if (ts.isIdentifier(node.left)) { + if (ts.isIdentifier(node.right)) { + if (shopifyAliases.has(node.right.text)) shopifyAliases.add(node.left.text) + if (interceptAliases.has(node.right.text)) interceptAliases.add(node.left.text) + } else if (isShopifyInterceptAccess(ts, node.right, shopifyAliases)) { + interceptAliases.add(node.left.text) + } + } + } + + ts.forEachChild(node, visit) + } + visit(analysis.sourceFile) + + return analysis.shopifyAliases.size + analysis.interceptAliases.size > before +} + +/** Collect import bindings and named exports (structural, computed once). */ +function collectImportsAndExports(ts: typeof import('typescript'), analysis: FileAnalysis, resolved: Map) { + const visit = (node: ts.Node): void => { + // import {intercept} from './x' ; import {intercept as foo} from './x' + if (ts.isImportDeclaration(node) && node.importClause && ts.isStringLiteral(node.moduleSpecifier)) { + const resolvedPath = resolved.get(node.moduleSpecifier.text) + if (resolvedPath) { + const named = node.importClause.namedBindings + if (named && ts.isNamedImports(named)) { + for (const spec of named.elements) { + const importedName = spec.propertyName?.text ?? spec.name.text + analysis.imports.set(spec.name.text, {resolvedPath, importedName}) + } + } + } + } + + // export {foo} ; export {foo as bar} + if (ts.isExportDeclaration(node) && node.exportClause && ts.isNamedExports(node.exportClause)) { + for (const spec of node.exportClause.elements) { + const localName = spec.propertyName?.text ?? spec.name.text + analysis.exports.set(spec.name.text, localName) + } + } + + // export const foo = ... + if (ts.isVariableStatement(node) && node.modifiers?.some((mod) => mod.kind === ts.SyntaxKind.ExportKeyword)) { + for (const decl of node.declarationList.declarations) { + if (ts.isIdentifier(decl.name)) analysis.exports.set(decl.name.text, decl.name.text) + } + } + + ts.forEachChild(node, visit) + } + visit(analysis.sourceFile) +} + +/** + * The real API is `shopify.intercept('', callback)`. A genuine intercept + * registration has a SECOND argument that is a function/callback. We accept: + * - arrow function shopify.intercept('x', () => {}) + * - function expression shopify.intercept('x', function () {}) + * - a reference to a function (identifier or property access) + * shopify.intercept('x', handler) / handlers.onX + * Anything else (missing, or a non-function literal like a string/number/object) + * is treated as a missing/invalid callback — a suspected malformed registration. + * Extra trailing args are tolerated (we only inspect args[0] and args[1]). + */ +function hasCallbackArg(ts: typeof import('typescript'), call: ts.CallExpression): boolean { + const second = call.arguments[1] + if (!second) return false + return ( + ts.isArrowFunction(second) || + ts.isFunctionExpression(second) || + ts.isIdentifier(second) || + ts.isPropertyAccessExpression(second) + ) +} + +/** Final pass: collect every intercept callsite using the resolved alias sets. */ +function collectCallsites(ts: typeof import('typescript'), analysis: FileAnalysis): InterceptCallsite[] { + const callsites: InterceptCallsite[] = [] + const {sourceFile, shopifyAliases, interceptAliases} = analysis + + const visit = (node: ts.Node): void => { + if (ts.isCallExpression(node)) { + const callee = node.expression + const isInterceptCall = + isShopifyInterceptAccess(ts, callee, shopifyAliases) || + (ts.isIdentifier(callee) && interceptAliases.has(callee.text)) + + if (isInterceptCall) { + const firstArg = node.arguments[0] + const callbackPresent = hasCallbackArg(ts, node) + const {line, character} = sourceFile.getLineAndCharacterOfPosition(node.getStart(sourceFile)) + const base = {file: analysis.path, line: line + 1, column: character + 1} + + if (!firstArg) { + callsites.push({ + ...base, + event: null, + argText: '', + unresolvedReason: 'intercept() called with no event argument', + }) + } else if (ts.isStringLiteralLike(firstArg)) { + // string literal or no-substitution template literal + if (callbackPresent) { + callsites.push({...base, event: firstArg.text, argText: firstArg.getText(sourceFile)}) + } else { + // Literal event but no callback — surface it, but do NOT count it as a + // resolved event: it's a suspected malformed intercept registration. + callsites.push({ + ...base, + event: null, + argText: firstArg.getText(sourceFile), + unresolvedReason: `intercept('${firstArg.text}', ...) is missing its callback argument (suspected malformed registration)`, + }) + } + } else { + callsites.push({ + ...base, + event: null, + argText: firstArg.getText(sourceFile), + unresolvedReason: `first argument is a ${ts.SyntaxKind[firstArg.kind]}, not a string literal`, + }) + } + } + } + ts.forEachChild(node, visit) + } + visit(sourceFile) + return callsites +} + +/** Resolve every import module specifier in a file to a local path. */ +async function resolveFileModuleSpecifiers( + ts: typeof import('typescript'), + sourceFile: ts.SourceFile, + filePath: string, +): Promise> { + const specifiers = new Set() + const visit = (node: ts.Node): void => { + if (ts.isImportDeclaration(node) && ts.isStringLiteral(node.moduleSpecifier)) { + specifiers.add(node.moduleSpecifier.text) + } + ts.forEachChild(node, visit) + } + visit(sourceFile) + + const resolved = new Map() + await Promise.all( + [...specifiers].map(async (spec) => { + const path = await resolveLocalModule(spec, filePath) + if (path) resolved.set(spec, path) + }), + ) + return resolved +} + +/** + * Detect all POS intercept events reachable (statically, control-flow-agnostic) + * from the given entry point(s), following the full local import graph. Accepts + * one or more entry modules (a target may declare more than one, and an + * extension may declare the intercept target more than once). + */ +export async function detectPosIntercepts( + entryFilePaths: string | string[], +): Promise { + const ts = await loadTypeScript() + + const entries = uniq((Array.isArray(entryFilePaths) ? entryFilePaths : [entryFilePaths]).filter(Boolean)) + + // Walk every entry's import graph, sharing a `visited` set so shared modules + // are analyzed once. + const visited = new Set() + const importedByEntry = await Promise.all(entries.map((entry) => findAllImportedFiles(entry, visited))) + const allFiles = uniq([...entries, ...importedByEntry.flat()]) + + // Parse + seed every file. + const analyses = new Map() + await Promise.all( + allFiles.map(async (filePath) => { + let content: string + try { + content = readFileSync(filePath).toString() + // eslint-disable-next-line no-catch-all/no-catch-all + } catch { + return + } + const sourceFile = ts.createSourceFile(filePath, content, ts.ScriptTarget.Latest, true, scriptKindFor(ts, filePath)) + const analysis: FileAnalysis = { + path: filePath, + sourceFile, + shopifyAliases: new Set([SHOPIFY_GLOBAL]), + interceptAliases: new Set(), + exports: new Map(), + imports: new Map(), + } + const resolved = await resolveFileModuleSpecifiers(ts, sourceFile, filePath) + collectImportsAndExports(ts, analysis, resolved) + analyses.set(filePath, analysis) + }), + ) + + // Whole-graph fixpoint: grow intra-file aliases, then propagate intercept + // aliases across re-exports/imports, until nothing changes. + let changed = true + while (changed) { + changed = false + for (const analysis of analyses.values()) { + // Intra-file fixpoint. + let grew = true + while (grew) { + grew = growAliases(ts, analysis) + if (grew) changed = true + } + // Cross-file: import of an exported intercept alias becomes an alias here. + for (const [localName, {resolvedPath, importedName}] of analysis.imports) { + if (analysis.interceptAliases.has(localName)) continue + const target = analyses.get(resolvedPath) + if (!target) continue + const targetLocal = target.exports.get(importedName) + if (targetLocal && target.interceptAliases.has(targetLocal)) { + analysis.interceptAliases.add(localName) + changed = true + } + } + } + } + + // Collect callsites across the graph. + const callsites: InterceptCallsite[] = [] + for (const analysis of analyses.values()) { + callsites.push(...collectCallsites(ts, analysis)) + } + + const events = uniq( + callsites.filter((callsite) => callsite.event !== null).map((callsite) => callsite.event as string), + ).sort() + const unresolved = callsites.filter((callsite) => callsite.event === null) + + return {events, callsites, unresolved, analyzedFiles: allFiles} +} + +/** + * Deploy-path entry: given the parsed extension configuration and directory, + * derive intercept events from the `pos.app.ready.data` target's module(s). + * Returns undefined when the extension declares no intercept-supporting target. + */ +export async function deriveInterceptsFromConfig( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + config: {targeting?: any; extension_points?: any} | undefined, + directory: string, +): Promise { + const entryModules = findInterceptEntryModules(config, directory) + if (entryModules.length === 0) return undefined + return detectPosIntercepts(entryModules) +} diff --git a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_comparison.test.ts b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_comparison.test.ts new file mode 100644 index 00000000000..15db8a2c8c5 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_comparison.test.ts @@ -0,0 +1,177 @@ +import {detectPosIntercepts} from './pos_intercept_detection.js' +import {detectPosInterceptsSimple, InterceptWarningKind} from './pos_intercept_detection_simple.js' +import {joinPath} from '@shopify/cli-kit/node/path' +import {fileURLToPath} from 'node:url' +import {dirname} from 'node:path' +import {describe, expect, test} from 'vitest' + +const currentDir = dirname(fileURLToPath(import.meta.url)) +const fixtures = joinPath(currentDir, 'fixtures', 'pos-intercept-compare') +const fixture = (name: string) => joinPath(fixtures, name) + +// --------------------------------------------------------------------------- +// THREE-WAY COMPARISON MATRIX: complex (full) detector vs "safe simplest". +// +// For each pattern we record three things: +// [C] complex RESOLVES the event +// [S] simple RESOLVES the event (direct string-literal calls only) +// [W] simple WARNS instead (kind of warning emitted) +// +// All calls use the real API shape: shopify.intercept('', callback). +// +// pattern | C resolves | S resolves | S warns +// -------------------------------------------------|------------|------------|-------------------- +// shopify.intercept('beforecheckout', cb) | yes | yes | — +// const {intercept}=shopify; intercept('x', cb) | yes | no | destructure +// const s=shopify; s.intercept('x', cb) | yes | no | object-alias-access +// let fn; fn=shopify.intercept; fn('x', cb) | yes | no | function-reference +// cross-file export const block=shopify.intercept | yes | no | function-reference +// if/else both branches | yes | yes | — +// dynamic variable first arg | unresolved | no | dynamic-arg +// literal event, NO callback (malformed) | unresolved | no | missing-callback +// ('event', cb, extraArg) trailing args tolerated | yes | yes | — +// const s=shopify; s.toast.show() (noise) | n/a | no | — (no false positive) +// +// KEY PROPERTY under test: the simple detector has ZERO SILENT MISSES on the +// alias patterns. When it fails to resolve, it WARNS — it never drops. +// --------------------------------------------------------------------------- + +interface Case { + name: string + file: string + /** Events the COMPLEX detector should resolve. */ + complexEvents: string[] + /** Events the SIMPLE detector should resolve (direct literals only). */ + simpleEvents: string[] + /** Warning kinds the SIMPLE detector should emit. */ + simpleWarnKinds: InterceptWarningKind[] +} + +const cases: Case[] = [ + { + name: 'direct call — C:resolve S:resolve (no warn)', + file: 'case-direct.ts', + complexEvents: ['beforecheckout'], + simpleEvents: ['beforecheckout'], + simpleWarnKinds: [], + }, + { + name: 'same-file destructure — C:resolve S:WARN(destructure)', + file: 'case-destructure.ts', + complexEvents: ['destructured'], + simpleEvents: [], + simpleWarnKinds: ['destructure'], + }, + { + name: 'object-alias s.intercept() — C:resolve S:WARN(object-alias-access)', + file: 'case-object-alias.ts', + complexEvents: ['objectalias'], + simpleEvents: [], + simpleWarnKinds: ['object-alias-access'], + }, + { + name: 'reassignment fn=shopify.intercept — C:resolve S:WARN(function-reference)', + file: 'case-reassign.ts', + complexEvents: ['reassigned'], + simpleEvents: [], + simpleWarnKinds: ['function-reference'], + }, + { + name: 'cross-file re-exported ref — C:resolve S:WARN(function-reference at creation site)', + file: 'case-crossfile.ts', + complexEvents: ['crossfile'], + simpleEvents: [], + simpleWarnKinds: ['function-reference'], + }, + { + name: 'alias chain const a=shopify; const b=a — C:resolve S:WARN(object-alias-access, chain closed)', + file: 'case-alias-chain.ts', + complexEvents: ['aliaschain'], + simpleEvents: [], + simpleWarnKinds: ['object-alias-access'], + }, + { + name: 'if/else both branches — C:resolve S:resolve (no warn)', + file: 'case-branches.ts', + complexEvents: ['branchelse', 'branchif'], + simpleEvents: ['branchelse', 'branchif'], + simpleWarnKinds: [], + }, + { + name: 'dynamic variable arg — C:unresolved S:WARN(dynamic-arg)', + file: 'case-dynamic.ts', + complexEvents: [], + simpleEvents: [], + simpleWarnKinds: ['dynamic-arg'], + }, + { + name: 'literal event but NO callback (malformed) — C:unresolved S:WARN(missing-callback)', + file: 'case-missing-callback.ts', + complexEvents: [], + simpleEvents: [], + simpleWarnKinds: ['missing-callback'], + }, + { + name: 'trailing args tolerated — C:resolve S:resolve (no warn)', + file: 'case-trailing-args.ts', + complexEvents: ['trailing'], + simpleEvents: ['trailing'], + simpleWarnKinds: [], + }, + { + name: 'noise: const s=shopify used for non-intercept — S: no resolve, NO false-positive warn', + file: 'case-alias-noise.ts', + complexEvents: [], + simpleEvents: [], + simpleWarnKinds: [], + }, +] + +describe('complex vs safe-simplest POS intercept detector — three-way matrix', () => { + cases.forEach((testCase) => { + test(testCase.name, async () => { + const [complex, simple] = await Promise.all([ + detectPosIntercepts(fixture(testCase.file)), + detectPosInterceptsSimple(fixture(testCase.file)), + ]) + + expect(complex.events).toEqual(testCase.complexEvents) + expect(simple.events).toEqual(testCase.simpleEvents) + expect(simple.warnings.map((warning) => warning.kind).sort()).toEqual([...testCase.simpleWarnKinds].sort()) + + // Every warning carries actionable location + raw source. + simple.warnings.forEach((warning) => { + expect(warning.line).toBeGreaterThan(0) + expect(warning.raw.length).toBeGreaterThan(0) + expect(warning.message).toContain('capabilities.intercepts') + }) + }) + }) + + test('malformed (literal event, no callback): BOTH surface it, NEITHER counts it as an event', async () => { + const [complex, simple] = await Promise.all([ + detectPosIntercepts(fixture('case-missing-callback.ts')), + detectPosInterceptsSimple(fixture('case-missing-callback.ts')), + ]) + // Not counted as a real event by either detector. + expect(complex.events).toEqual([]) + expect(simple.events).toEqual([]) + // But surfaced by both — complex as an unresolved callsite, simple as a warning. + expect(complex.unresolved.some((cs) => /missing its callback/.test(cs.unresolvedReason ?? ''))).toBe(true) + expect(simple.warnings.map((warning) => warning.kind)).toEqual(['missing-callback']) + }) + + test('KEY PROPERTY: zero silent misses — simple either resolves or warns for every alias pattern', async () => { + const aliasPatterns = cases.filter((testCase) => testCase.simpleWarnKinds.length > 0 || testCase.complexEvents.length > 0) + for (const testCase of aliasPatterns) { + // eslint-disable-next-line no-await-in-loop + const simple = await detectPosInterceptsSimple(fixture(testCase.file)) + const covered = simple.events.length > 0 || simple.warnings.length > 0 + // Skip the parity-only "noise" cases; here we assert intercept-bearing + // patterns are never silently dropped. + if (testCase.complexEvents.length > 0 || testCase.simpleWarnKinds.length > 0) { + expect(covered, `"${testCase.name}" should resolve or warn, never silently miss`).toBe(true) + } + } + }) +}) diff --git a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_report.test.ts b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_report.test.ts new file mode 100644 index 00000000000..e858afa00de --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_report.test.ts @@ -0,0 +1,206 @@ +import {detectPosIntercepts} from './pos_intercept_detection.js' +import {detectPosInterceptsSimple} from './pos_intercept_detection_simple.js' +import {mkdtempSync, mkdirSync, writeFileSync} from 'node:fs' +import {tmpdir} from 'node:os' +import {join, dirname} from 'node:path' +import {test} from 'vitest' + +// --------------------------------------------------------------------------- +// RUNNABLE REPORT (not an assertion suite — it prints and exits 0). +// +// Shows the SIMPLE vs COMPLEX POS intercept detectors side-by-side over a +// curated set of samples that tell the story, including cases where the COMPLEX +// detector ALSO fails (its shallow data-flow has real limits). +// +// RUN: +// cd packages/app && ../../node_modules/.bin/vitest run \ +// src/cli/models/extensions/specifications/pos_intercept_detection_report.test.ts +// +// Optional: run against a real file instead of the built-in samples: +// REPORT_PATH=/abs/path/to/entry.ts ../../node_modules/.bin/vitest run \ +// src/cli/models/extensions/specifications/pos_intercept_detection_report.test.ts +// --------------------------------------------------------------------------- + +interface Sample { + name: string + label: string + /** Minimal code shown in the report (just enough to understand the sample). */ + show: string + /** entry filename within the sample dir. */ + entry: string + /** filename -> source content (verbatim) actually written + analyzed. */ + files: {[filename: string]: string} +} + +const CB = '() => {}' + +const SAMPLES: Sample[] = [ + { + name: 'plain direct call', + label: 'BOTH RESOLVE', + show: `shopify.intercept('beforecheckout', cb)`, + entry: 'index.ts', + files: { + 'index.ts': `declare const shopify: {intercept: (e: string, cb: () => void) => void}\nshopify.intercept('beforecheckout', ${CB})\n`, + }, + }, + { + name: 'same-file destructure', + label: 'simple WARNS, complex RESOLVES', + show: `const {intercept} = shopify\nintercept('beforediscount', cb)`, + entry: 'index.ts', + files: { + 'index.ts': `declare const shopify: {intercept: (e: string, cb: () => void) => void}\nconst {intercept} = shopify\nintercept('beforediscount', ${CB})\n`, + }, + }, + { + name: 'object alias then member call', + label: 'simple WARNS, complex RESOLVES', + show: `const s = shopify\ns.intercept('beforeexchange', cb)`, + entry: 'index.ts', + files: { + 'index.ts': `declare const shopify: {intercept: (e: string, cb: () => void) => void}\nconst s = shopify\ns.intercept('beforeexchange', ${CB})\n`, + }, + }, + { + name: 'cross-file re-exported reference', + label: 'simple WARNS, complex RESOLVES', + show: `// dep.ts\nexport const block = shopify.intercept\n// index.ts\nblock('beforecancel', cb)`, + entry: 'index.ts', + files: { + 'dep.ts': `declare const shopify: {intercept: (e: string, cb: () => void) => void}\nexport const block = shopify.intercept\n`, + 'index.ts': `import {block} from './dep.js'\nblock('beforecancel', ${CB})\n`, + }, + }, + { + name: 'alias chain', + label: 'simple WARNS (chain closed), complex RESOLVES', + show: `const a = shopify\nconst b = a\nb.intercept('beforecapture', cb)`, + entry: 'index.ts', + files: { + 'index.ts': `declare const shopify: {intercept: (e: string, cb: () => void) => void}\nconst a = shopify\nconst b = a\nb.intercept('beforecapture', ${CB})\n`, + }, + }, + { + name: 'dynamic event argument (variable)', + label: 'BOTH UNRESOLVED (dynamic) — simple WARNS', + show: `const evt = 'beforetax'\nshopify.intercept(evt, cb)`, + entry: 'index.ts', + files: { + 'index.ts': `declare const shopify: {intercept: (e: string, cb: () => void) => void}\nconst evt = 'beforetax'\nshopify.intercept(evt, ${CB})\n`, + }, + }, + { + name: 'const-folded event name', + label: 'COMPLEX ALSO FAILS (no constant folding) — simple WARNS', + show: `const E = 'beforeshipping'\nshopify.intercept(E, cb)`, + entry: 'index.ts', + files: { + 'index.ts': `declare const shopify: {intercept: (e: string, cb: () => void) => void}\nconst E = 'beforeshipping'\nshopify.intercept(E, ${CB})\n`, + }, + }, + { + name: 'higher-order passing', + label: 'COMPLEX ALSO FAILS (silent miss on HOF) — simple WARNS', + show: `function register(fn) { fn('beforepayment', cb) }\nregister(shopify.intercept)`, + entry: 'index.ts', + files: { + 'index.ts': `declare const shopify: {intercept: (e: string, cb: () => void) => void}\nfunction register(fn: typeof shopify.intercept) {\n fn('beforepayment', ${CB})\n}\nregister(shopify.intercept)\n`, + }, + }, + { + name: 'stored in object then called', + label: 'COMPLEX ALSO FAILS (silent miss via object storage) — simple WARNS', + show: `const m = {i: shopify.intercept}\nm.i('beforerefund', cb)`, + entry: 'index.ts', + files: { + 'index.ts': `declare const shopify: {intercept: (e: string, cb: () => void) => void}\nconst m = {i: shopify.intercept}\nm.i('beforerefund', ${CB})\n`, + }, + }, +] + +function writeSample(sample: Sample): string { + const dir = mkdtempSync(join(tmpdir(), 'pos-intercept-report-')) + for (const [filename, content] of Object.entries(sample.files)) { + const filePath = join(dir, filename) + mkdirSync(dirname(filePath), {recursive: true}) + writeFileSync(filePath, content) + } + return join(dir, sample.entry) +} + +type Mark = '✅' | '🟠' | '❌' + +/** ✅ resolved · 🟠 surfaced/warned but not resolved · ❌ silent miss. */ +function symbolFor(resolved: boolean, surfaced: boolean): Mark { + if (resolved) return '✅' + if (surfaced) return '🟠' + return '❌' +} + +function indent(text: string): string { + return text + .replace(/\n+$/, '') + .split('\n') + .map((line) => ` ${line}`) + .join('\n') +} + +interface Tally { + '✅': number + '🟠': number + '❌': number +} + +async function reportOne( + heading: string, + entryPath: string, + code: string | undefined, + tally: {simple: Tally; complex: Tally}, +): Promise { + const [simple, complex] = await Promise.all([detectPosInterceptsSimple(entryPath), detectPosIntercepts(entryPath)]) + + const simpleSymbol = symbolFor(simple.events.length > 0, simple.warnings.length > 0) + const complexSymbol = symbolFor(complex.events.length > 0, complex.unresolved.length > 0) + tally.simple[simpleSymbol]++ + tally.complex[complexSymbol]++ + + const lines: string[] = [] + lines.push(heading) + if (code !== undefined) lines.push(indent(code)) + lines.push(` SIMPLE ${simpleSymbol} COMPLEX ${complexSymbol}`) + lines.push('') + // eslint-disable-next-line no-console + console.log(lines.join('\n')) +} + +test('POS intercept detector report (simple vs complex)', async () => { + const overridePath = process.env.REPORT_PATH + const tally = { + simple: {'✅': 0, '🟠': 0, '❌': 0}, + complex: {'✅': 0, '🟠': 0, '❌': 0}, + } + // eslint-disable-next-line no-console + console.log( + `\n\nPOS INTERCEPT DETECTOR REPORT — simple (safe-simplest) vs complex (alias-resolving)\n` + + `✅ resolved · 🟠 not resolved but surfaced/warned · ❌ silent miss (returned nothing)\n`, + ) + + if (overridePath) { + await reportOne(`FILE: ${overridePath}`, overridePath, undefined, tally) + } else { + for (let index = 0; index < SAMPLES.length; index++) { + const sample = SAMPLES[index]! + const entryPath = writeSample(sample) + // eslint-disable-next-line no-await-in-loop + await reportOne(`${index + 1}. ${sample.name}`, entryPath, sample.show, tally) + } + } + + const row = (t: Tally) => `✅ ${t['✅']} 🟠 ${t['🟠']} ❌ ${t['❌']}` + // eslint-disable-next-line no-console + console.log( + `${'─'.repeat(56)}\nTALLY\n SIMPLE : ${row(tally.simple)}\n COMPLEX: ${row(tally.complex)}\n` + + `${'─'.repeat(56)}\nSimple has ZERO ❌ (never a silent miss); complex trades some 🟠/❌\nfor silent derivation of the alias cases.\n`, + ) +}) diff --git a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_simple.ts b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_simple.ts new file mode 100644 index 00000000000..fd987b52db5 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_simple.ts @@ -0,0 +1,294 @@ +import {findInterceptEntryModules, InterceptCallsite} from './pos_intercept_detection.js' +import {findAllImportedFiles} from './type-generation.js' +import {readFileSync} from '@shopify/cli-kit/node/fs' +import {uniq} from '@shopify/cli-kit/common/array' +import type ts from 'typescript' + +// --------------------------------------------------------------------------- +// SPIKE / PROTOTYPE: the "SAFE SIMPLEST" POS intercept detector. +// +// Comparison baseline for the full detector in pos_intercept_detection.ts. +// +// The guiding rule: NEVER silently under-report. A call is either +// (a) DERIVED SILENTLY — a direct `shopify.intercept('')`, or +// (b) FLAGGED LOUDLY — anything that *could* be an intercept but that this +// simple scan can't statically read. +// +// It does NOT resolve or follow anything indirect. It does NOT track aliases, +// reassignments, cross-file references, or HOF passing. Instead it recognizes +// those patterns SYNTACTICALLY and emits a warning (file:line + raw source) +// telling the developer to declare that intercept explicitly in the TOML. +// +// Resolved events come ONLY from direct string-literal `shopify.intercept(...)`. +// +// Warned patterns (detected, never resolved): +// * destructure: const {intercept} = shopify / {intercept: x} +// * function-reference: const f = shopify.intercept ; x = shopify.intercept ; +// wrap(shopify.intercept) (any non-call use of the fn) +// * object-alias-access: const s = shopify; ... s.intercept(...) (call or ref) +// * dynamic-arg: shopify.intercept(, cb) +// * missing-callback: shopify.intercept('literal') with no callback +// +// The real API is `shopify.intercept('', callback)`. A direct literal +// call is DERIVED only when a callback second arg is present; a literal call +// with no callback is flagged (missing-callback) as a suspected malformed +// registration rather than counted. Extra trailing args are tolerated. +// +// Control flow is ignored (every direct literal call counts, any branch). +// --------------------------------------------------------------------------- + +async function loadTypeScript(): Promise { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const mod: any = await import('typescript') + return mod.default ?? mod +} + +function scriptKindFor(ts: typeof import('typescript'), filePath: string): ts.ScriptKind { + if (filePath.endsWith('.ts')) return ts.ScriptKind.TS + if (filePath.endsWith('.tsx')) return ts.ScriptKind.TSX + return ts.ScriptKind.JSX +} + +export type InterceptWarningKind = + | 'destructure' + | 'function-reference' + | 'object-alias-access' + | 'dynamic-arg' + | 'missing-callback' + +/** + * A genuine intercept registration has a callback as its SECOND argument. + * Accepts arrow fn, function expression, or a function reference (identifier / + * property access). See the full detector for the shared rationale. + */ +function hasCallbackArg(ts: typeof import('typescript'), call: ts.CallExpression): boolean { + const second = call.arguments[1] + if (!second) return false + return ( + ts.isArrowFunction(second) || + ts.isFunctionExpression(second) || + ts.isIdentifier(second) || + ts.isPropertyAccessExpression(second) + ) +} + +/** An intercept-shaped pattern the simple scan can't statically resolve. */ +export interface InterceptWarning { + kind: InterceptWarningKind + file: string + line: number + column: number + /** Raw source text of the flagged node, for the developer message. */ + raw: string + message: string +} + +export interface SimpleDetectionResult { + /** Events resolved from direct string-literal calls ONLY. */ + events: string[] + /** The direct string-literal callsites that produced `events`. */ + callsites: InterceptCallsite[] + /** Indirect/dynamic patterns flagged for explicit TOML declaration. */ + warnings: InterceptWarning[] + analyzedFiles: string[] +} + +const DECLARE_HINT = 'Declare this intercept explicitly under capabilities.intercepts in the extension TOML.' + +/** + * Collect identifiers aliased to the `shopify` global WITHIN this file: + * `const s = shopify` / `s = shopify`, plus same-file alias CHAINS + * (`const a = shopify; const b = a`). This is a cheap same-file fixpoint over + * identifier-to-identifier assignments — NOT the complex detector's whole-graph, + * cross-file data-flow. It exists so the "never silently miss" guarantee holds: + * without chain handling, `b.intercept(...)` would be neither resolved nor + * warned. Aliases are used ONLY to scope the object-alias-access warning — a + * bare `const s = shopify` used for unrelated reasons still never warns; we flag + * only when `.intercept` is actually accessed on the alias. + */ +function collectShopifyAliases(ts: typeof import('typescript'), sourceFile: ts.SourceFile): Set { + const aliases = new Set() + + // One syntactic pass, applied to a fixpoint so chains resolve regardless of + // declaration order. + const pass = (): boolean => { + const before = aliases.size + const visit = (node: ts.Node): void => { + if ( + ts.isVariableDeclaration(node) && + ts.isIdentifier(node.name) && + node.initializer && + ts.isIdentifier(node.initializer) && + (node.initializer.text === 'shopify' || aliases.has(node.initializer.text)) + ) { + aliases.add(node.name.text) + } + if ( + ts.isBinaryExpression(node) && + node.operatorToken.kind === ts.SyntaxKind.EqualsToken && + ts.isIdentifier(node.left) && + ts.isIdentifier(node.right) && + (node.right.text === 'shopify' || aliases.has(node.right.text)) + ) { + aliases.add(node.left.text) + } + ts.forEachChild(node, visit) + } + visit(sourceFile) + return aliases.size > before + } + while (pass()) { + // repeat until no new aliases are discovered + } + return aliases +} + +function analyzeFileSimple( + ts: typeof import('typescript'), + filePath: string, +): {callsites: InterceptCallsite[]; warnings: InterceptWarning[]} { + let content: string + try { + content = readFileSync(filePath).toString() + // eslint-disable-next-line no-catch-all/no-catch-all + } catch { + return {callsites: [], warnings: []} + } + const sourceFile = ts.createSourceFile(filePath, content, ts.ScriptTarget.Latest, true, scriptKindFor(ts, filePath)) + const aliases = collectShopifyAliases(ts, sourceFile) + const callsites: InterceptCallsite[] = [] + const warnings: InterceptWarning[] = [] + + const posOf = (node: ts.Node) => { + const {line, character} = sourceFile.getLineAndCharacterOfPosition(node.getStart(sourceFile)) + return {file: filePath, line: line + 1, column: character + 1} + } + const truncate = (text: string) => (text.length > 80 ? `${text.slice(0, 77)}...` : text).replace(/\s+/g, ' ') + + const visit = (node: ts.Node): void => { + // 1. Destructure off shopify (or a shopify alias): const {intercept} = shopify + if ( + ts.isVariableDeclaration(node) && + ts.isObjectBindingPattern(node.name) && + node.initializer && + ts.isIdentifier(node.initializer) && + (node.initializer.text === 'shopify' || aliases.has(node.initializer.text)) + ) { + for (const element of node.name.elements) { + const propName = + element.propertyName && ts.isIdentifier(element.propertyName) + ? element.propertyName.text + : ts.isIdentifier(element.name) + ? element.name.text + : undefined + if (propName === 'intercept') { + warnings.push({ + kind: 'destructure', + ...posOf(node), + raw: truncate(node.getText(sourceFile)), + message: `intercept is destructured off shopify; the simple detector can't read the event name(s). ${DECLARE_HINT}`, + }) + } + } + } + + // 2/3/4. Any `.intercept` where obj is `shopify` or a shopify alias. + if ( + ts.isPropertyAccessExpression(node) && + node.name.text === 'intercept' && + ts.isIdentifier(node.expression) + ) { + const objName = node.expression.text + const isShopify = objName === 'shopify' + const isAlias = aliases.has(objName) + if (isShopify || isAlias) { + const parent = node.parent + const isDirectCallee = parent && ts.isCallExpression(parent) && parent.expression === node + + if (isShopify && isDirectCallee) { + // Direct `shopify.intercept('', callback)`. + const call = parent as ts.CallExpression + const firstArg = call.arguments[0] + const callbackPresent = hasCallbackArg(ts, call) + if (firstArg && ts.isStringLiteralLike(firstArg)) { + if (callbackPresent) { + callsites.push({...posOf(call), event: firstArg.text, argText: firstArg.getText(sourceFile)}) + } else { + warnings.push({ + kind: 'missing-callback', + ...posOf(call), + raw: truncate(call.getText(sourceFile)), + message: `shopify.intercept('${firstArg.text}') is missing its callback argument (suspected malformed registration). ${DECLARE_HINT}`, + }) + } + } else { + warnings.push({ + kind: 'dynamic-arg', + ...posOf(call), + raw: truncate(call.getText(sourceFile)), + message: `shopify.intercept called with a non-string-literal event argument. ${DECLARE_HINT}`, + }) + } + } else if (isShopify) { + // Function reference: const f = shopify.intercept / x = ... / HOF arg. + warnings.push({ + kind: 'function-reference', + ...posOf(node), + raw: truncate((node.parent ?? node).getText(sourceFile)), + message: `A reference to shopify.intercept is taken; the simple detector can't follow it. ${DECLARE_HINT}`, + }) + } else { + // Access on a shopify-object alias: s.intercept(...) or ref to s.intercept. + warnings.push({ + kind: 'object-alias-access', + ...posOf(node), + raw: truncate((node.parent ?? node).getText(sourceFile)), + message: `intercept is accessed via an alias of the shopify object; the simple detector can't read the event. ${DECLARE_HINT}`, + }) + } + } + } + + ts.forEachChild(node, visit) + } + visit(sourceFile) + return {callsites, warnings} +} + +/** + * Safe-simplest detector: resolves ONLY direct string-literal + * `shopify.intercept('x')` calls; flags every other intercept-shaped pattern as + * a warning rather than silently missing it. Target-scoped import graph. + */ +export async function detectPosInterceptsSimple( + entryFilePaths: string | string[], +): Promise { + const ts = await loadTypeScript() + const entries = uniq((Array.isArray(entryFilePaths) ? entryFilePaths : [entryFilePaths]).filter(Boolean)) + + const visited = new Set() + const importedByEntry = await Promise.all(entries.map((entry) => findAllImportedFiles(entry, visited))) + const allFiles = uniq([...entries, ...importedByEntry.flat()]) + + const callsites: InterceptCallsite[] = [] + const warnings: InterceptWarning[] = [] + for (const filePath of allFiles) { + const {callsites: fileCallsites, warnings: fileWarnings} = analyzeFileSimple(ts, filePath) + callsites.push(...fileCallsites) + warnings.push(...fileWarnings) + } + + const events = uniq(callsites.map((cs) => cs.event as string)).sort() + return {events, callsites, warnings, analyzedFiles: allFiles} +} + +/** Deploy-path parity with the full detector: config-driven, target-scoped. */ +export async function deriveInterceptsFromConfigSimple( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + config: {targeting?: any; extension_points?: any} | undefined, + directory: string, +): Promise { + const entryModules = findInterceptEntryModules(config, directory) + if (entryModules.length === 0) return undefined + return detectPosInterceptsSimple(entryModules) +} diff --git a/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.test.ts b/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.test.ts new file mode 100644 index 00000000000..362f4db5555 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.test.ts @@ -0,0 +1,49 @@ +import {deriveAndMergeIntercepts} from './pos_ui_extension.js' +import {POS_INTERCEPT_TARGET} from './pos_intercept_detection.js' +import {joinPath} from '@shopify/cli-kit/node/path' +import {fileURLToPath} from 'node:url' +import {dirname} from 'node:path' +import {describe, expect, test} from 'vitest' + +const currentDir = dirname(fileURLToPath(import.meta.url)) +const fixtureDir = joinPath(currentDir, 'fixtures', 'pos-intercept-demo') + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +const configWith = (capabilities?: any): any => ({ + name: 'demo', + capabilities, + targeting: [ + {target: POS_INTERCEPT_TARGET, module: 'src/index.ts'}, + {target: 'pos.home.tile.render', module: 'src/home-tile.ts'}, + ], +}) + +describe('deriveAndMergeIntercepts (deploy path)', () => { + test('emits derived events into capabilities.intercepts when TOML declares none', async () => { + // Given no declared intercepts, derivation alone populates the field the + // backend reads. + const result = await deriveAndMergeIntercepts(configWith(undefined), fixtureDir) + + expect(result?.intercepts).toContain('beforecheckout') + expect(result?.intercepts).toContain('beforecapture') + // Render-target event must not leak in. + expect(result?.intercepts).not.toContain('shouldnotappear') + }) + + test('unions declared and derived events (declaration keeps working)', async () => { + const result = await deriveAndMergeIntercepts(configWith({intercepts: ['manuallydeclared']}), fixtureDir) + + expect(result?.intercepts).toContain('manuallydeclared') + expect(result?.intercepts).toContain('beforecheckout') + // De-duplicated + sorted. + expect(result?.intercepts).toEqual([...new Set(result?.intercepts)].sort()) + }) + + test('falls back to declared capabilities when no intercept target is declared', async () => { + const result = await deriveAndMergeIntercepts( + {name: 'demo', capabilities: {intercepts: ['onlydeclared']}, targeting: []}, + fixtureDir, + ) + expect(result?.intercepts).toEqual(['onlydeclared']) + }) +}) diff --git a/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.ts b/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.ts index 75da3471ab6..e42a0ba5ffe 100644 --- a/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.ts +++ b/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.ts @@ -1,9 +1,11 @@ import {getDependencyVersion} from '../../app/app.js' import {createExtensionSpecification} from '../specification.js' -import {BaseSchema, CapabilitiesSchema} from '../schemas.js' +import {BaseSchema, CapabilitiesSchema, NewExtensionPointsSchema} from '../schemas.js' import {ExtensionInstance} from '../extension-instance.js' +import {deriveInterceptsFromConfig} from './pos_intercept_detection.js' import {BugError} from '@shopify/cli-kit/node/error' import {zod} from '@shopify/cli-kit/node/schema' +import {outputDebug, outputWarn} from '@shopify/cli-kit/node/output' const dependency = '@shopify/retail-ui-extensions' @@ -45,8 +47,58 @@ type PosUIConfigType = zod.infer const PosUISchema = BaseSchema.extend({ name: zod.string(), capabilities: PosCapabilitiesSchema.optional(), + // Declared mount points. Each target carries a `module` path; intercept + // derivation reads the `pos.app.ready.data` target's module as its entry. + targeting: NewExtensionPointsSchema.optional(), }) +type PosCapabilities = zod.infer + +/** + * SPIKE: derive intercept events from the extension source and merge them into + * the capabilities object emitted at deploy time. + * + * The derived events are unioned with any hand-authored `capabilities.intercepts` + * so both the derivation-first and the TOML-declaration workflows produce the + * same shape. Unresolved (dynamic) event args are logged as a warning — they are + * a known reliability gap and are never silently dropped. + * + * Detection failures are non-fatal: we fall back to whatever the TOML declared + * so a source-parsing edge case can never block a deploy. + */ +export async function deriveAndMergeIntercepts( + config: PosUIConfigType | undefined, + directory: string, +): Promise { + const capabilities = config?.capabilities + let derivedEvents: string[] = [] + try { + const detection = await deriveInterceptsFromConfig(config, directory) + if (detection) { + derivedEvents = detection.events + if (detection.unresolved.length > 0) { + const locations = detection.unresolved + .map((callsite) => `${callsite.file}:${callsite.line} (${callsite.argText || ''})`) + .join(', ') + outputWarn( + `POS intercept detection could not statically resolve ${detection.unresolved.length} intercept event(s): ${locations}. ` + + `Declare these explicitly under capabilities.intercepts if the host must know about them.`, + ) + } + outputDebug(`Derived POS intercept events from source: ${derivedEvents.join(', ') || '(none)'}`) + } + // eslint-disable-next-line no-catch-all/no-catch-all + } catch (error) { + outputWarn(`POS intercept source detection failed; falling back to declared capabilities.intercepts. ${error}`) + } + + const declaredEvents = capabilities?.intercepts ?? [] + const mergedEvents = [...new Set([...declaredEvents, ...derivedEvents])].sort() + + if (mergedEvents.length === 0) return capabilities + return {...capabilities, intercepts: mergedEvents} +} + const posUISpec = createExtensionSpecification({ identifier: 'pos_ui_extension', dependency, @@ -62,11 +114,24 @@ const posUISpec = createExtensionSpecification({ deployConfig: async (config, directory) => { const result = await getDependencyVersion(dependency, directory) if (result === 'not_found') throw new BugError(`Dependency ${dependency} not found`) + + // Derive intercept events from the extension's SOURCE CODE (control-flow + // agnostic AST walk of the pos.app.ready.data target's import graph) and + // fold them into `capabilities.intercepts`. This is the transmission + // mechanism for derived events: there is no dedicated wire field. + // deployConfig emits the SAME `capabilities.intercepts` array the backend + // already reads — whether an event came from the TOML or from source, it + // lands in the deployed version config identically, so the backend persists + // it as a capability with no backend change required. Derived + TOML-declared + // events are unioned so a hand-authored declaration keeps working alongside + // derivation. + const capabilities = await deriveAndMergeIntercepts(config, directory) + return { name: config.name, description: config.description, renderer_version: result?.version, - capabilities: config.capabilities, + capabilities, } }, })