chore: clean package surfaces and source artifacts#47
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughRestructures the monorepo into a multi-package workspace (ignite-core, ignite-adapters, ignite-renderer, ignite-element), migrates from root-level src to packages, implements an effects-based event model moving emit from commands to effects, adds headless agent runtime APIs for deterministic testing and automation, introduces schema introspection, and extensively revises documentation and configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant User as Application Code
participant Element as ignite-element<br/>(Web Component)
participant Core as ignite-core<br/>(Projection Factory)
participant Adapter as Adapter<br/>(XState/Redux/MobX)
participant UI as Browser/DOM
User->>Element: Create component with<br/>igniteCore({view, commands,<br/>events, effects})
Element->>Core: createProjectionFactory(<br/>adapter, options)
Core->>Core: Build view resolver,<br/>command facade,<br/>event emitter
Element->>Adapter: Create adapter factory
Adapter-->>Element: IgniteAdapter instance
Element->>Element: Attach shadow DOM,<br/>store projection & adapter
User->>UI: User interaction
UI->>Element: DOM event fired
Element->>Element: Command handler invoked<br/>from facade
Element->>Adapter: actor.send(event)
Adapter->>Adapter: State update,<br/>compute new snapshot
Adapter-->>Element: notify listeners
Element->>Core: Resolve view from<br/>current snapshot
Core-->>Element: Derived view object
Element->>Element: Effects callback<br/>(snapshot, prevSnapshot, {emit,select})
Element->>Element: emit(eventType, payload)
Element->>Element: Dispatch CustomEvent<br/>on host
Element->>UI: Re-render via strategy<br/>(jsx/lit) with view props
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
✅ Created PR with unit tests: #48 |
There was a problem hiding this comment.
Actionable comments posted: 17
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
docs/site/src/content/docs/index.mdx (1)
102-110:⚠️ Potential issue | 🟡 MinorLive-demo links and GitHub path are inconsistent with the updated toggle example.
The page teaches
my-toggle, but both the StackBlitz and GitHub links still reference the counter example, confusing users during onboarding. Additionally, the GitHub repository link path is missing the monorepo package prefix.
- StackBlitz URLs should reference
my-toggle.tsxinstead ofmy-counter.tsx- GitHub link should be
packages/ignite-element/src/examples/xstateinstead ofsrc/examples/xstateSuggested fixes
- [Open in StackBlitz](https://stackblitz.com/edit/ignite-element?file=src%2Fmy-counter.tsx) (inline playground) + [Open in StackBlitz](https://stackblitz.com/edit/ignite-element?file=src%2Fmy-toggle.tsx) (inline playground) - [Open repo example](https://github.com/0xjcf/ignite-element/tree/main/src/examples/xstate) + [Open repo example](https://github.com/0xjcf/ignite-element/tree/main/packages/ignite-element/src/examples/xstate) - data-src="https://stackblitz.com/edit/ignite-element?embed=1&file=src%2Fmy-counter.tsx&view=both" + data-src="https://stackblitz.com/edit/ignite-element?embed=1&file=src%2Fmy-toggle.tsx&view=both"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/site/src/content/docs/index.mdx` around lines 102 - 110, Update the two StackBlitz links and the embedded iframe so they point to my-toggle.tsx instead of my-counter.tsx, and change the GitHub example link to include the monorepo package prefix by using packages/ignite-element/src/examples/xstate; specifically update the hrefs and the iframe data-src/url that reference "src%2Fmy-counter.tsx" to "src%2Fmy-toggle.tsx" and replace the GitHub path "src/examples/xstate" with "packages/ignite-element/src/examples/xstate" to keep the live demo and repo link consistent with the my-toggle example.packages/ignite-element/src/IgniteElementFactory.ts (2)
168-177:⚠️ Potential issue | 🟠 MajorRuntime acquisition never releases shared adapters or facade effects.
resolveRuntimeResources()eagerly createsruntimeHost/runtimeAdditionalArgsand flipssharedRuntimeActive = true, but neitherreleaseSharedResources()nor any runtime unsubscribe path clears that flag or runscleanupAdditionalArgs(runtimeAdditionalArgs). Sincepackages/ignite-element/src/runtime/agent.tsroutes evengetState(),getView(), andgetSchema()throughresolveRuntime(), one runtime read makescleanupineffective and can keep Ignite-owned shared actors plus anyeffects(...)subscription alive for the rest of the module lifetime.Also applies to: 181-213
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ignite-element/src/IgniteElementFactory.ts` around lines 168 - 177, resolveRuntimeResources() sets up runtimeHost/runtimeAdditionalArgs and flips sharedRuntimeActive=true but releaseSharedResources() and runtime teardown paths never clear that flag or call cleanupAdditionalArgs(runtimeAdditionalArgs), causing runtime reads to permanently keep shared actors/effects alive; update releaseSharedResources(), the runtime unsubscribe/teardown path, and resolveRuntime-related cleanup to call cleanupAdditionalArgs(runtimeAdditionalArgs) (using the same runtimeAdditionalArgs captured by resolveRuntimeResources()) and set sharedRuntimeActive = false, and ensure any early/unsubscribe paths from resolveRuntime or runtimeHost also invoke that cleanup so sharedRuntimeActive is reliably cleared and runtimeAdditionalArgs are cleaned up when the runtime is released.
80-92:⚠️ Potential issue | 🟠 MajorWiden the return type to include agent runtime methods.
The function augments
registerwithcreateAgentRuntime(...)viaObject.assign(lines 309–323), but the declared return typeComponentFactory<...>omits methods likeexecute,getState,getView,getSchema,on,watch, andwatchView. SinceigniteElementFactoryis exported in the public API (index.ts:20), callers won't have type support for these methods even though they exist at runtime.Update the return type to include
IgniteAgentRuntime<State, Events, SchemaState, View>or a compatible intersection so that TypeScript properly reflects the augmented interface.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ignite-element/src/IgniteElementFactory.ts` around lines 80 - 92, The public factory function igniteElementFactory currently returns ComponentFactory<State, Event, RenderArgs, View> but is augmented at runtime (via Object.assign on register using createAgentRuntime) with agent runtime methods (execute, getState, getView, getSchema, on, watch, watchView); update the declared return type of igniteElementFactory to an intersection that includes IgniteAgentRuntime<State, Event, SchemaState?, View> (or the concrete IgniteAgentRuntime generic) so callers get proper typings — locate the function igniteElementFactory and its return type, and change it to ComponentFactory<...> & IgniteAgentRuntime<...> (or an equivalent compatible union/intersection) ensuring the generic parameters align with the existing State, Event, and View generics.
🟡 Minor comments (12)
docs/migrations/v2.2.3-effects-events.md-22-34 (1)
22-34:⚠️ Potential issue | 🟡 Minor"Before" example doesn't show the
emitcall being migrated.The summary states the old pattern had "command bodies [that] both updated state and emitted DOM events," but the "Before" code example doesn't include an
emit()call. This may confuse readers about what they're migrating away from.📝 Suggested improvement
const registerToggle = igniteCore({ source: machine, events: (event) => ({ toggled: event<{ isOn: boolean }>(), }), - commands: ({ actor }) => ({ + commands: ({ actor, emit }) => ({ toggle: () => { actor.send({ type: "TOGGLE" }); + emit("toggled", { isOn: true }); // ← this pattern is being removed }, }), });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/migrations/v2.2.3-effects-events.md` around lines 22 - 34, The "Before" example omits the old emit call—update the registerToggle/igniteCore snippet so the commands.toggle body demonstrates the legacy pattern by both updating state and calling emit (or emit DOM event) in addition to actor.send; reference the registerToggle constant, the commands.toggle function, actor.send, and the events.toggled name so readers can see the pre-migration emit usage that the migration text describes.scripts/migrate-emit-to-effects.mjs-191-194 (1)
191-194:⚠️ Potential issue | 🟡 MinorValidate
--reportargument exists before accessing.If
--reportis passed as the last argument without a path,args[reportIndex + 1]will beundefined, andwriteFilewill fail with an unhelpful error.🛡️ Proposed fix
if (reportIndex !== -1) { - reportPath = args[reportIndex + 1]; + reportPath = args[reportIndex + 1]; + if (!reportPath) { + process.stderr.write("Error: --report requires a file path argument\n"); + process.exitCode = 1; + return; + } args.splice(reportIndex, 2); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/migrate-emit-to-effects.mjs` around lines 191 - 194, The code accesses args[reportIndex + 1] without validating that a value exists, which can produce undefined and later cause writeFile to fail; update the block that handles reportIndex (use the identifiers reportIndex, reportPath, args) to first check that reportIndex !== -1 and reportIndex + 1 < args.length before assigning reportPath and splicing args, and if the path is missing either emit a clear error and exit or set a sensible default so writeFile isn't called with undefined.docs/api/README.md-33-45 (1)
33-45:⚠️ Potential issue | 🟡 MinorClarify wording to avoid return-type ambiguity.
Line 33 says
igniteCore(...)“also returns a headless runtime”, while Line 25 documents a callable registration return. Please clarify that the returned value is the same callable with headless runtime capabilities attached.✏️ Suggested wording update
-`igniteCore(...)` also returns a headless runtime for deterministic testing and automation: +The value returned by `igniteCore(...)` is the registration function and also exposes a headless runtime for deterministic testing and automation:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/api/README.md` around lines 33 - 45, Update the README wording to remove ambiguity by stating that igniteCore(...) returns the same callable registration function but augmented with a headless runtime interface (e.g., the exported helper igniteTest shown) rather than a separate object; mention that this helper wraps execute() so the callable you get from igniteCore is still invokable as before and additionally exposes headless testing methods like .given(), .when(), .expectState(), and .expectEvent().docs/site/src/content/docs/migration/effects-events.mdx-72-72 (1)
72-72:⚠️ Potential issue | 🟡 MinorRelease note narrows command context too much.
“Commands now receive
{ actor, host }only” is XState-specific. The docs elsewhere describe adapter-specific surfaces (actor/store/observable) plushost.Suggested wording
-`emit` has been removed from command context. Commands now receive `{ actor, host }` only, and typed event emission belongs in `effects()`. +`emit` has been removed from command context. Commands receive adapter controls (for example `actor`, `store`, or `observable`) plus `host`, and typed event emission belongs in `effects()`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/site/src/content/docs/migration/effects-events.mdx` at line 72, The release note sentence incorrectly narrows the command context by saying "Commands now receive { actor, host } only"; update the wording around `emit` and command contexts to reflect adapter-specific surfaces instead—mention that command handlers may receive `{ actor, host }`, or adapter variants like `{ store, host }` or `{ observable, host }` as appropriate rather than a universal `{ actor, host }` restriction, and clarify that typed event emission belongs in `effects()` (not in command context); adjust the sentence referencing `emit`, `commands`, and `effects()` to list the adapter-specific possibilities (`actor` / `store` / `observable`) plus `host`.docs/ignite-query.md-214-241 (1)
214-241:⚠️ Potential issue | 🟡 MinorUse
viewin thisigniteCoreexample.The rest of this PR has already moved projected-facade examples to
view, but this new guide still teachesstates. Shipping the new docs with the older hook name makes the public surface look inconsistent immediately.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/ignite-query.md` around lines 214 - 241, The example creates usersCore with igniteCore using the old projected-facade hook name states; replace the states property with view while keeping the same projection logic and identifiers (i.e., change states: ({ snapshot, matchState }) => ({ ... }) to view: ({ snapshot, matchState }) => ({ mode: matchState(...), users: snapshot.context.data ?? [], isFetching: snapshot.context.isFetching, errorMessage: snapshot.context.status === "error" ? String(snapshot.context.error) : null }), leaving commands: ({ actor }) => ({ refetch: ..., retry: ... }) and usersActor/usersCore names untouched so the example matches the rest of the docs.docs/ignite-web3.md-168-193 (1)
168-193:⚠️ Potential issue | 🟡 MinorSwitch this example to
viewas well.This guide is otherwise aligned with the new runtime model, but the projection example still uses
statesinstead of the newerviewhook used elsewhere in the PR. That makes the freshly added docs inconsistent with the current facade terminology.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/ignite-web3.md` around lines 168 - 193, The example uses the old projection hook "states" instead of the new "view" API; update the projection in the igniteCore call for walletCore by renaming the "states" property to "view" (keep the function signature and returned shape intact—i.e., view: ({ snapshot, matchState }) => ({ mode: matchState(...), address: snapshot.context.address, chainId: snapshot.context.chainId, errorMessage: snapshot.context.error ?? null })); leave the "commands" block and other symbols (walletCore, igniteCore, snapshot, matchState, commands) unchanged so the example aligns with the new runtime model and terminology.docs/site/src/content/docs/concepts/state-adapters.mdx-43-45 (1)
43-45:⚠️ Potential issue | 🟡 MinorClarify the supported Redux source shapes.
packages/ignite-adapters/src/igniteCore/redux.tscurrently accepts aSlice, a() => EnhancedStore, or an existing store. Calling this a “slice (factory)” suggests a slice factory is supported, but those functions are routed through the store-factory branch instead.Suggested wording
-- **Redux Toolkit**: pass a slice (factory) or a store instance; Ignite subscribes and reads state on change. If you pass your own store, Ignite will unsubscribe its adapter as elements disconnect but will not dispose your store. +- **Redux Toolkit**: pass a slice, a store factory, or a store instance; Ignite subscribes and reads state on change. If you pass your own store, Ignite will unsubscribe its adapter as elements disconnect but will not dispose your store.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/site/src/content/docs/concepts/state-adapters.mdx` around lines 43 - 45, The docs text incorrectly suggests a "slice (factory)" shape; update the Redux bullet to list the exact supported source shapes accepted by the adapter: a Slice, a store factory (a () => EnhancedStore), or an existing store instance, and note that slice factory functions are routed through the store-factory branch in the implementation (see redux.ts handling for store-factory vs slice branches). Replace "slice (factory)" with the clearer wording "a Slice, a store factory, or a store instance" and add a short note that externally created stores are subscribed to but not disposed by Ignite.packages/ignite-core/src/runtime/effects.ts-84-126 (1)
84-126:⚠️ Potential issue | 🟡 MinorKeep
prevSnapshotadvancing even when effects throw.If the callback throws, Line 126 never runs. The next notification is then compared against an older baseline, so
prevSnapshotandselect(...).changedcan describe the wrong transition.Suggested fix
- if (isObjectStyleEffectsCallback(effects)) { - ( - effects as FacadeEffectsObjectCallback< - Snapshot, - CommandActor, - Events, - Host - > - )({ - snapshot, - prevSnapshot, - actor, - emit, - host, - select, - }); - } else { - (effects as FacadeEffectsCallback<Snapshot, CommandActor, Events, Host>)( - snapshot, - prevSnapshot, - { - actor, - emit, - host, - select, - }, - ); - } - prevSnapshot = snapshot; + try { + if (isObjectStyleEffectsCallback(effects)) { + ( + effects as FacadeEffectsObjectCallback< + Snapshot, + CommandActor, + Events, + Host + > + )({ + snapshot, + prevSnapshot, + actor, + emit, + host, + select, + }); + } else { + ( + effects as FacadeEffectsCallback< + Snapshot, + CommandActor, + Events, + Host + > + )(snapshot, prevSnapshot, { + actor, + emit, + host, + select, + }); + } + } finally { + prevSnapshot = snapshot; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ignite-core/src/runtime/effects.ts` around lines 84 - 126, The subscription callback can throw while invoking the effects, preventing prevSnapshot from advancing; wrap the effects invocation in a try/finally (inside the adapter.subscribe callback after creating snapshot, actor, and select) so that prevSnapshot = snapshot always runs in the finally block; locate the logic around resolveSnapshot, resolveActor, createSelect, isObjectStyleEffectsCallback and effects and move the assignment of prevSnapshot into the finally to ensure select(...).changed uses the correct baseline on the next notification.packages/ignite-element/src/runtime/agent.ts-4-23 (1)
4-23:⚠️ Potential issue | 🟡 MinorRename the generic
Eventtype to avoid shadowing the DOMEventtype.The generic type parameter
Event(lines 4, 14, 25) shadows the DOMEventtype. On lines 68 and 100, the listener parameters are annotated asevent: Event, referring to the adapter event type, not the DOM event. Theas CustomEvent<unknown>andas EventListenercasts suppress type checking around the listener bodies, allowing the code to compile despite the type mismatch between what the DOM provides and what the listener parameter expects.Also applies to: 25-34
docs/site/src/content/docs/api/ignite-core.mdx-32-43 (1)
32-43:⚠️ Potential issue | 🟡 MinorThis example reverses the cleanup ownership contract.
Line 32 says user-provided sources are never stopped, but this example says
cleanup: true“assert[s] ownership of a passed actor.” As written, it tells readers the opposite lifecycle behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/site/src/content/docs/api/ignite-core.mdx` around lines 32 - 43, The docs contradict themselves: the bullets state "user-provided sources are never stopped" while the code example shows cleanup: true and claims it asserts ownership of a passed actor. Fix by making the behavior consistent: either change the explanatory bullet about cleanup to state that setting cleanup: true causes Ignite to take ownership and stop a user-provided source (update the sentence referencing cleanup and lifecycle.browser), or change the example to use cleanup: false and adjust its comment to reflect non-ownership; reference the symbols cleanup, lifecycle.browser, igniteCore, source, and actor when updating the text and the code example so both description and example match the intended ownership semantics.packages/ignite-element/package.json-164-165 (1)
164-165:⚠️ Potential issue | 🟡 MinorPotential issue:
postreleasescript runs afterreleasewhich already publishes.The
releasescript runschangeset publish(which publishes to npm), and thenpostreleaserunspnpm publishagain. This could cause a duplicate publish attempt or version conflict error.🐛 Proposed fix
"release:beta": "node ./scripts/release-beta.mjs", "release": "changeset publish", - "postrelease": "pnpm publish", "prepare": "husky"If
postreleaseis intended for a different purpose (e.g., publishing to a secondary registry), consider renaming it or documenting its intent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ignite-element/package.json` around lines 164 - 165, The package.json defines a "postrelease" script that runs "pnpm publish" after the "release" flow already invokes "changeset publish", causing a duplicate publish attempt; update package.json by either removing or renaming the "postrelease" script (or changing its command) so it doesn't run "pnpm publish" after the "release" pipeline, or document/condition it if it's intended for a different registry; locate the "postrelease" and "release" npm scripts in package.json (look for the "postrelease" and "release" entries and the "changeset publish" command) and make the appropriate change to avoid double publishing.packages/ignite-element/src/IgniteCore.ts-319-354 (1)
319-354:⚠️ Potential issue | 🟡 MinorDuplicate overload signature for
igniteCoreRedux.Lines 319-330 and 331-354 define the same implementation signature twice. The first is an overload declaration, but the second immediately follows with an identical signature and the actual implementation. This appears to be unintentional duplication.
🐛 Proposed fix: Remove duplicate overload
export function igniteCoreRedux( options: ReduxConfig, ): IgniteCoreReturn< InferStateAndEvent<ReduxBlueprintSource | ReduxInstanceSource>["State"], InferStateAndEvent<ReduxBlueprintSource | ReduxInstanceSource>["Event"], InferStateAndEvent<ReduxBlueprintSource | ReduxInstanceSource>["State"], Record<string, unknown>, ReduxCommandActorFor<ReduxBlueprintSource | ReduxInstanceSource>, FacadeCommandResult, EventMap ->; - -export function igniteCoreRedux( - options: ReduxConfig, -): IgniteCoreReturn< - InferStateAndEvent<ReduxBlueprintSource | ReduxInstanceSource>["State"], - InferStateAndEvent<ReduxBlueprintSource | ReduxInstanceSource>["Event"], - InferStateAndEvent<ReduxBlueprintSource | ReduxInstanceSource>["State"], - Record<string, unknown>, - ReduxCommandActorFor<ReduxBlueprintSource | ReduxInstanceSource>, - FacadeCommandResult, - EventMap -> { +> { const projection = igniteCoreReduxProjection(options);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ignite-element/src/IgniteCore.ts` around lines 319 - 354, Remove the duplicated overload declaration for igniteCoreRedux: there are two identical function type signatures followed by the implementation; keep a single overload + implementation combo and delete the redundant declaration (remove the first/earlier duplicate). Locate the declarations for igniteCoreRedux and ensure the remaining version calls igniteCoreReduxProjection(...) and returns bindProjectionToElements(... ) as the typed IgniteCoreReturn; do not change function bodies or types, only remove the extra duplicate signature.
🧹 Nitpick comments (19)
configs/tsconfig/base.json (1)
1-15: LGTM!Solid base TypeScript configuration with appropriate strictness checks and modern ES2020 target. The combination of
strict,isolatedModules, and unused-code checks provides good type safety and code quality enforcement.Optional: Consider Node16/NodeNext module resolution for better ESM support.
The current
moduleResolution: "Node"uses the legacy algorithm. For modern projects with native ESM support, consider upgrading to"Node16"or"NodeNext"in a future iteration, which provides better alignment with Node.js ESM resolution semantics.📦 Optional module resolution upgrade
{ "compilerOptions": { "target": "ES2020", "module": "ESNext", - "moduleResolution": "Node", + "moduleResolution": "NodeNext", "strict": true,Note: This change may require adjustments to import paths and package.json exports across the monorepo.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@configs/tsconfig/base.json` around lines 1 - 15, The config currently sets "moduleResolution": "Node" which uses the legacy resolution algorithm; to opt into modern ESM-aware resolution change the moduleResolution value to "Node16" or "NodeNext" in the tsconfig's compilerOptions (update the "moduleResolution" key in configs/tsconfig/base.json), then run type checks and fix any import/path or package.json exports that break due to the new ESM resolution semantics.biome.json (1)
16-17: Narrow.d.tsignores to generated paths only.Line 16 and Line 17 exclude all declaration files, which can skip checks for authored source declarations. Prefer scoping these ignores to generated directories (for example
dist/**).♻️ Suggested adjustment
- "!**/*.d.ts", - "!**/*.d.ts.map", + "!**/dist/**/*.d.ts", + "!**/dist/**/*.d.ts.map",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@biome.json` around lines 16 - 17, The current glob ignores "!**/*.d.ts" and "!**/*.d.ts.map" are too broad and exclude authored declaration files; narrow them to generated output dirs by replacing those patterns with ignores scoped to build artifacts (for example change "!**/*.d.ts" -> "!dist/**/*.d.ts" and "!**/*.d.ts.map" -> "!dist/**/*.d.ts.map" or another generated directory like "build/**" used by your toolchain) so only generated declaration files are skipped while source .d.ts files remain checked.scripts/migrate-emit-to-effects.mjs (1)
44-60: Brace matching is naive and may produce false positives.The
findMatchingBracefunction counts braces without accounting for those inside string literals, template literals, comments, or regex. This could cause incorrect body boundary detection. Since the tool is intentionally non-destructive (producing reports rather than rewriting), this is acceptable, but worth documenting.Consider adding a brief inline comment noting this limitation:
function findMatchingBrace(source, openIndex) { + // Note: naive counting; braces in strings/comments may cause offset. let depth = 0;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/migrate-emit-to-effects.mjs` around lines 44 - 60, The brace-matching in findMatchingBrace is simplistic and ignores braces inside string literals, template literals, comments and regexes which can produce false positives; add a brief inline comment directly above the findMatchingBrace function that documents this limitation (mention strings, template literals, comments, regexes) and that the script is intentionally non-destructive/report-only so this behavior is acceptable.docs/architecture.md (1)
163-189: Clarify that the enforcement paths are examples for consuming projects.The file paths in the "Enforcement" section (e.g.,
src/sigil-machine.ts,src/actors/,src/*.tsx) appear to be illustrative examples for a project consuming the ignite libraries, not paths within this monorepo itself. Consider adding a brief note clarifying these are example paths that should be adapted to each project's structure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/architecture.md` around lines 163 - 189, Update the "Enforcement" section in docs/architecture.md to make clear the listed file paths (e.g., `src/sigil-machine.ts`, `src/actors/`, `src/*.tsx`, and the Quick checks) are example paths for downstream/consuming projects and not literal paths inside this monorepo; add a single clarifying sentence (immediately after the enforcement list or before "Quick checks") that instructs consumers to adapt those paths/commands to their own project structure and tooling (refer to the "Enforcement" heading and the specific example entries `src/sigil-machine.ts`, `src/actors/`, `src/create-character.ts`, `src/rarityFromCharacter.ts`, and the "Quick checks" rg commands).packages/ignite-element/src/examples/xstate/package.json (1)
8-8: Consider live CSS rebuild duringdev.Running
build:cssonce before Vite can leave Tailwind output stale while editing styles. If live style iteration is expected, switch to a watch/parallel setup.Possible script adjustment
"scripts": { - "dev": "pnpm run build:css && vite", + "dev:css": "tailwindcss -i ./styles.css -o ./dist/styles.css --watch", + "dev": "pnpm run dev:css & vite", "build": "vite build" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ignite-element/src/examples/xstate/package.json` at line 8, The current "dev" script runs "build:css" once then starts Vite, which causes Tailwind output to go stale during editing; create a watch-style CSS task (e.g., "build:css:watch" that runs your Tailwind/PostCSS build in watch mode) and update the "dev" script to run that watch task in parallel with "vite" (use a tool like concurrently, npm-run-all --parallel, or pnpm's built-in parallel execution) so CSS rebuilds live while Vite serves.docs/site/src/content/docs/getting-started/project-setup.mdx (1)
14-19: Consider making “Optional advanced setup” a real section heading.This improves scanability and keeps structure consistent with docs-style sections.
💡 Suggested tweak
-Optional advanced setup: +## Optional advanced setup - add `ignite.config.ts` if you want project-wide renderer defaults, shared styles, or diagnostics - add the Vite/Webpack plugin only if you want the config file auto-loaded by your bundler - configure `lit` only if you want the non-default renderer path🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/site/src/content/docs/getting-started/project-setup.mdx` around lines 14 - 19, Turn the inline "Optional advanced setup:" line into a real section heading so it becomes scannable and consistent with other docs sections; replace the plain text line with a heading (e.g., "## Optional advanced setup" or the same heading level used in surrounding content) and ensure the following bullet list remains directly under that heading; update any adjacent TOC or headings if needed to preserve document structure.packages/ignite-element/src/tests/config.test.ts (1)
95-108: Test descriptions could be updated to reflect the new assertion style.The test names mention "invokes setGlobalStyles" (line 95) and "does not invoke setGlobalStyles" (line 104), but the assertions now check
getGlobalStyles()state rather than verifying invocation. Consider updating the test descriptions to match the behavioral assertions, e.g., "stores global styles when provided" and "does not store global styles when omitted".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ignite-element/src/tests/config.test.ts` around lines 95 - 108, Update the two test descriptions to reflect that they assert state via globalStylesModule.getGlobalStyles() rather than checking invocation; specifically, rename the "invokes setGlobalStyles when styles are provided" test to something like "stores global styles when provided" and rename "does not invoke setGlobalStyles when styles are omitted" to "does not store global styles when omitted" (tests reference defineIgniteConfig and globalStylesModule.getGlobalStyles).packages/ignite-element/src/types/schema.ts (1)
9-13: ConstrainStateto schema-safe values.
IgniteAgentSchema<State = IgniteSchemaValue>currently allows arbitraryStatetypes, which can bypass the intended schema contract.Type-safety tightening
-export type IgniteAgentSchema<State = IgniteSchemaValue> = { +export type IgniteAgentSchema<State extends IgniteSchemaValue = IgniteSchemaValue> = { commands: string[]; events: string[]; state: State; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ignite-element/src/types/schema.ts` around lines 9 - 13, The generic State parameter on IgniteAgentSchema can accept arbitrary types; tighten it by constraining State to the schema-safe union/shape (IgniteSchemaValue) so consumers cannot bypass the schema contract—update the IgniteAgentSchema type declaration (the type named IgniteAgentSchema and its generic State parameter) to require State extends IgniteSchemaValue while keeping the default generic to IgniteSchemaValue.packages/ignite-element/src/examples/xstate/README.md (1)
85-87: Prefer a concrete isolatedeffectsexample over placeholders.Using the real body here reduces migration ambiguity and avoids missing the
emitpattern ineffects.Suggested doc snippet update
- effects: (snapshot, prevSnapshot, ctx) => { - /* same effects as shared */ - }, + effects: (snapshot, prevSnapshot, { emit }) => { + if (!prevSnapshot) return; + if (snapshot.context.darkMode === prevSnapshot.context.darkMode) return; + emit("toggled", { isDark: snapshot.context.darkMode }); + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ignite-element/src/examples/xstate/README.md` around lines 85 - 87, Replace the placeholder effects: (snapshot, prevSnapshot, ctx) => { /* same effects as shared */ } with a concrete, isolated example that mirrors the shared behavior: implement the effects function body using snapshot, prevSnapshot and ctx, demonstrate the emit pattern (e.g., ctx.emit('eventName', payload)), show common operations such as diffing snapshot vs prevSnapshot, updating ctx state, and returning any cleanup/teardown if applicable; ensure the example is self-contained and clearly documents the role of snapshot, prevSnapshot and ctx so readers can copy it without needing external references.docs/site/src/content/docs/migration/v2.mdx (1)
77-79: Consider leading with the default lifecycle example.Line 71 says
cleanupis usually an override, but Line 77 demonstratescleanup: falsedirectly, which can be cargo-culted.Doc example tweak
-const shared = igniteCore({ source: actor, cleanup: false, view: /* ... */ }); +const shared = igniteCore({ source: actor, view: /* ... */ }); +// Override only if you intentionally manage teardown: +// const shared = igniteCore({ source: actor, cleanup: false, view: /* ... */ });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/site/src/content/docs/migration/v2.mdx` around lines 77 - 79, The example shows igniteCore being called with cleanup: false which contradicts the earlier note that cleanup is usually an override; change the example to demonstrate the default lifecycle first (omit the cleanup option when calling igniteCore with source: actor and view) and then add a second example that explicitly shows how to override cleanup (igniteCore({... , cleanup: false}) and the corresponding actor.stop() usage). Update the examples referencing igniteCore, shared, cleanup, actor, and Renderer so the first example uses the default call and the second example demonstrates the override.packages/ignite-element/tsconfig.typecheck.json (1)
3-33: Make the typecheck config explicitlynoEmit.This widens
rootDirto..and includes sibling packages, sotsc -pcan write artifacts across the workspace. The config chain (typecheck.json→tsconfig.json→lib.dom.json→lib.json→base.json) lacks an explicitnoEmitsetting. Pinning it here makes the "typecheck only" contract self-contained.🔧 Proposed fix
"compilerOptions": { "rootDir": "..", "baseUrl": ".", + "noEmit": true, "paths": {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ignite-element/tsconfig.typecheck.json` around lines 3 - 33, Add an explicit "noEmit": true to the compilerOptions in tsconfig.typecheck.json so the typecheck-only config cannot emit artifacts despite widening rootDir and including sibling packages; update the existing compilerOptions block (where "rootDir": "..", "baseUrl": ".", and "paths": {...} are defined) to include the "noEmit" flag to make the typecheck contract self-contained.packages/ignite-element/src/renderers/resolveConfiguredRenderStrategy.ts (1)
10-14: Consider removing test-only reset API from the public surface.
clearRegisteredRenderStrategiesForTestsis useful internally, but exposing it to all consumers broadens accidental foot-gun surface.Possible surface cleanup
export { - clearRegisteredRenderStrategiesForTests, getRegisteredRenderStrategies, registerRenderStrategy, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ignite-element/src/renderers/resolveConfiguredRenderStrategy.ts` around lines 10 - 14, Remove the test-only reset API from the module's public exports: stop exporting clearRegisteredRenderStrategiesForTests from resolveConfiguredRenderStrategy.ts so it isn't part of the public surface; keep getRegisteredRenderStrategies and registerRenderStrategy exported. Locate the export block that currently lists clearRegisteredRenderStrategiesForTests and remove that identifier (leaving the other exports unchanged), and ensure any internal tests import the function via a test-only path or use a non-exported internal accessor instead of the public export..fas-config.json (1)
20-20: Seedarchitecture-rules.jsonto re-enable drift detection.Given the current empty rules file, it’s worth adding a minimal rule set so boundary/drift checks provide signal in CI.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.fas-config.json at line 20, The repository references an empty architecture rules file via the "architectureRulesFile" entry (".fas/architecture-rules.json"), so add a minimal seeded rules JSON to that file to re-enable CI drift/boundary detection; create .fas/architecture-rules.json containing a small rule set (e.g., allowed layer dependencies, at least one rule for service->domain or package boundaries, and a default deny) so the detector has meaningful checks and CI will surface drift.packages/ignite-core/src/utils/matchState.ts (1)
12-16: Consider deterministic precedence when multiple case keys match.If both broad and specific keys can match the same snapshot, current behavior depends on object insertion order. Consider documenting this explicitly or preferring most-specific keys first.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ignite-core/src/utils/matchState.ts` around lines 12 - 16, The matching loop in matchState.ts currently returns the first matching entry based on object insertion order, which is non-deterministic when both broad and specific keys match; update the logic in the matchState function to enforce deterministic precedence by ordering case keys by specificity before iterating (e.g., sort Object.keys(cases) so more-specific keys come first — for example by descending key length or by number of state segments — then call snapshot.matches on each ordered key), or alternatively add a clear JSDoc note on the matchState function describing that callers must order cases from specific to broad; reference the existing variables/symbols: cases, snapshot.matches, and the return type Cases.packages/ignite-element/src/igniteCore/mobxEntry.ts (1)
20-63: Reuse the shared projection binder instead of copying it here.
createDomEmitplus the localbindProjectionToElementsduplicate the same host guard / DOM event wiring that already exists inpackages/ignite-element/src/createComponentFactory.ts. Keeping a second copy makes future runtime changes easy to miss in the MobX entrypoint and is also what forces the cast below to paper over the local typing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ignite-element/src/igniteCore/mobxEntry.ts` around lines 20 - 63, Remove the local duplicate DOM event wiring and host-guard by deleting createDomEmit and bindProjectionToElements and instead import and call the shared projection binder from packages/ignite-element/src/createComponentFactory.ts (the exported binder that wraps igniteElementFactory). Replace the local call sites that used bindProjectionToElements(projection) with the shared binder invocation, passing projection.createAdapter, projection.scope, projection.cleanup, projection.eventTypes, projection.resolveView and projection.createAdditionalArgs as required so the shared host guard and DOM emit wiring are reused and the type-casts used to paper over the duplication are no longer necessary.README.md (1)
299-306: Give the projection and JSX files different basenames.Having both
toggle.view.tsandtoggle.view.tsxmakes extensionless imports ambiguous for people who copy this layout. Renaming one of them (toggle.projection.ts,toggle.component.tsx, etc.) would make the recommended structure safer to follow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 299 - 306, The README lists both toggle.view.ts and toggle.view.tsx which creates ambiguous extensionless imports; rename one file (e.g., toggle.view.ts -> toggle.projection.ts or toggle.view.tsx -> toggle.component.tsx) and update all references in the example and any sample imports to the new basename so extensionless imports are unambiguous; ensure the README example tree and any code snippets that reference toggle.view or toggle.view.tsx are updated to the chosen new name.packages/ignite-element/src/types/agent.ts (1)
76-77: Type mismatch:watchViewhandler receivesViewbut usesIgniteAgentStateListener<View>.The
IgniteAgentStateListener<State>type expects(state: State, prevState: State)parameters, which is semantically named for state. While the type works correctly forView, the naming could be confusing. Consider either:
- Renaming
IgniteAgentStateListenertoIgniteAgentChangeListener, or- Adding a separate
IgniteAgentViewListeneralias for clarityThis is a minor naming nit; the functionality is correct.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ignite-element/src/types/agent.ts` around lines 76 - 77, The watcher API uses IgniteAgentStateListener for both state and view callbacks which is confusing because the listener name implies "state"; update the types so intent is clear by either renaming IgniteAgentStateListener to IgniteAgentChangeListener across the codebase (including the watch and watchView declarations) or by adding a new alias IgniteAgentViewListener = IgniteAgentStateListener<View> and switching watchView(handler: IgniteAgentStateListener<View>) to watchView(handler: IgniteAgentViewListener) to preserve behavior while clarifying intent; ensure you update all references (watchView, watch, and the type declaration) consistently.packages/ignite-element/src/createComponentFactory.ts (1)
256-293: Consider extracting the repeated type parameter list.The generic type parameters for
createComponentFactoryWithRendererandcreateComponentFactoryare nearly identical and quite extensive. While this works correctly, the repetition increases maintenance burden.💡 Optional: Extract common type constraints
You could define a shared type alias for the common constraint pattern:
type ComponentFactoryTypeParams< State, Event, Snapshot, StatesResult extends Record<string, unknown> = Record<never, never>, CommandActor = { send: (event: Event) => void; getState: () => State }, CommandsResult extends FacadeCommandResult = Record<never, FacadeCommandFunction>, Additional extends Record<string, unknown> = Record<never, never>, View = TemplateResult | IgniteJsxChild, Events extends EventMap = EmptyEventMap, > = { /* marker type */ };This is optional since the current approach is explicit and works correctly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ignite-element/src/createComponentFactory.ts` around lines 256 - 293, The long repeated generic parameter list used by createComponentFactory and createComponentFactoryWithRenderer should be extracted into a shared type alias (e.g., ComponentFactoryTypeParams) capturing the common type parameters and constraints; create that alias and replace the repeated generic lists in both function signatures with a single alias reference, update any internal references (State, Event, Snapshot, StatesResult, CommandActor, CommandsResult, Additional, View, Events) to use the alias types, and ensure exported types/uses compile unchanged while reducing duplication.packages/ignite-element/package.json (1)
189-195: Consider marking optional peer dependencies withpeerDependenciesMeta.All state management libraries (
@reduxjs/toolkit,mobx,xstate) are listed as required peer dependencies, but users typically only use one. Marking them as optional prevents unnecessary installation warnings.📦 Proposed fix to add peerDependenciesMeta
"peerDependencies": { "@reduxjs/toolkit": ">=2.3.0", "lit-html": ">=3.2.1", "mobx": ">=6.13.5", "redux": ">=5.0.1", "xstate": ">=5.19.0" }, + "peerDependenciesMeta": { + "@reduxjs/toolkit": { "optional": true }, + "mobx": { "optional": true }, + "redux": { "optional": true }, + "xstate": { "optional": true } + }, "engines": {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ignite-element/package.json` around lines 189 - 195, The package.json currently lists `@reduxjs/toolkit`, mobx, and xstate as required peerDependencies but they are mutually optional for consumers; add a peerDependenciesMeta object to package.json that sets "@reduxjs/toolkit", "mobx", and "xstate" to {"optional": true} so they no longer trigger mandatory peer warnings; locate the peerDependencies block and add the peerDependenciesMeta alongside it referencing those exact package names.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: aa5630a3-fa7c-4663-b6c2-66bf4af9632e
⛔ Files ignored due to path filters (5)
packages/ignite-element/src/examples/mobx/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpackages/ignite-element/src/examples/redux/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpackages/ignite-element/src/examples/xstate/package-lock.jsonis excluded by!**/package-lock.jsonpackages/ignite-element/src/examples/xstate/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (243)
.fas-config.json.fas/TASKS.md.fas/tasks/add-effects-aligned-testing-dsl-for-ignite-components.md.fas/tasks/add-ignitecore-agent-schema-generation.md.fas/tasks/clean-package-surfaces-remove-generated-source-artifacts-and.md.fas/tasks/collapse-duplicate-projection-and-component-factory-assembly.md.fas/tasks/finish-ignitecore-migration-path-for-effects-based-events.md.fas/tasks/formalize-deterministic-replay-semantics-for-ignite-effects.md.fas/tasks/move-xstate-integration-out-of-ignite-core-and-make-ignite-c.md.fas/tasks/remove-deprecated-emit-from-commands-and-finalize-command-on.md.fas/tasks/rename-ignite-store-to-ignite-adapters-and-normalize-adapter.md.github/workflows/ci.yml.gitignore.husky/commit-msgREADME.mdbiome.jsonconfigs/tsconfig/base.jsonconfigs/tsconfig/lib.dom.jsonconfigs/tsconfig/lib.jsonconfigs/tsconfig/lib.node.jsonconfigs/vite/lib.tsdocs/api/README.mddocs/architecture.mddocs/ignite-query.mddocs/ignite-web3.mddocs/ignite-websocket.mddocs/migrations/v2.2.3-effects-events.mddocs/site/src/content/docs/api/ignite-core.mdxdocs/site/src/content/docs/concepts/events-and-commands.mdxdocs/site/src/content/docs/concepts/renderers.mdxdocs/site/src/content/docs/concepts/state-adapters.mdxdocs/site/src/content/docs/getting-started/first-component.mdxdocs/site/src/content/docs/getting-started/installation.mdxdocs/site/src/content/docs/getting-started/project-setup.mdxdocs/site/src/content/docs/guides/testing.mdxdocs/site/src/content/docs/index.mdxdocs/site/src/content/docs/migration/effects-events.mdxdocs/site/src/content/docs/migration/v2.mdxdocs/site/src/content/docs/overview/what-is-ignite-element.mdxdocs/site/src/styles/theme.cssdocs/testing.mddocs/when-code-becomes-cheap.mdpackage.jsonpackages/ignite-adapters/README.mdpackages/ignite-adapters/package.jsonpackages/ignite-adapters/src/adapters/MobxAdapter.tspackages/ignite-adapters/src/adapters/ReduxAdapter.tspackages/ignite-adapters/src/adapters/XStateAdapter.tspackages/ignite-adapters/src/igniteCore/mobx.tspackages/ignite-adapters/src/igniteCore/redux.tspackages/ignite-adapters/src/index.tspackages/ignite-adapters/src/mobx.tspackages/ignite-adapters/src/redux.tspackages/ignite-adapters/src/types.tspackages/ignite-adapters/src/utils/adapterGuards.tspackages/ignite-adapters/src/utils/igniteRedux.tspackages/ignite-adapters/src/utils/mobxGuards.tspackages/ignite-adapters/src/xstate.tspackages/ignite-adapters/tsconfig.jsonpackages/ignite-adapters/tsconfig.typecheck.jsonpackages/ignite-adapters/vite.config.tspackages/ignite-core/README.mdpackages/ignite-core/package.jsonpackages/ignite-core/src/IgniteAdapter.tspackages/ignite-core/src/RenderArgs.tspackages/ignite-core/src/createProjectionFactory.tspackages/ignite-core/src/events.tspackages/ignite-core/src/index.tspackages/ignite-core/src/runtime/effects.tspackages/ignite-core/src/types.tspackages/ignite-core/src/utils/isFunction.tspackages/ignite-core/src/utils/matchState.tspackages/ignite-core/tsconfig.jsonpackages/ignite-core/vite.config.tspackages/ignite-element/CHANGELOG.mdpackages/ignite-element/LICENSEpackages/ignite-element/README.mdpackages/ignite-element/package.jsonpackages/ignite-element/playwright.config.tspackages/ignite-element/src/IgniteAdapter.tspackages/ignite-element/src/IgniteCore.tspackages/ignite-element/src/IgniteElement.tspackages/ignite-element/src/IgniteElementFactory.tspackages/ignite-element/src/RenderArgs.tspackages/ignite-element/src/adapters/MobxAdapter.tspackages/ignite-element/src/adapters/ReduxAdapter.tspackages/ignite-element/src/adapters/XStateAdapter.tspackages/ignite-element/src/config.tspackages/ignite-element/src/config/loadIgniteConfig.tspackages/ignite-element/src/config/vite.tspackages/ignite-element/src/config/webpack.tspackages/ignite-element/src/createComponentFactory.tspackages/ignite-element/src/events.tspackages/ignite-element/src/examples/mobx/README.mdpackages/ignite-element/src/examples/mobx/another-counter-mobx.csspackages/ignite-element/src/examples/mobx/ignite.config.tspackages/ignite-element/src/examples/mobx/index.htmlpackages/ignite-element/src/examples/mobx/mobxCounterStore.tspackages/ignite-element/src/examples/mobx/mobxExample.tspackages/ignite-element/src/examples/mobx/package.jsonpackages/ignite-element/src/examples/mobx/theme.csspackages/ignite-element/src/examples/mobx/vite.config.tspackages/ignite-element/src/examples/redux/README.mdpackages/ignite-element/src/examples/redux/ignite.config.tspackages/ignite-element/src/examples/redux/package.jsonpackages/ignite-element/src/examples/redux/src/index.htmlpackages/ignite-element/src/examples/redux/src/js/reduxCounterStore.tspackages/ignite-element/src/examples/redux/src/js/reduxExample.tsxpackages/ignite-element/src/examples/redux/src/scss/styles.scsspackages/ignite-element/src/examples/redux/tsconfig.jsonpackages/ignite-element/src/examples/redux/vite.config.tspackages/ignite-element/src/examples/xstate/README.mdpackages/ignite-element/src/examples/xstate/advancedCounterMachine.tspackages/ignite-element/src/examples/xstate/ignite.config.tspackages/ignite-element/src/examples/xstate/index.htmlpackages/ignite-element/src/examples/xstate/package.jsonpackages/ignite-element/src/examples/xstate/styles.csspackages/ignite-element/src/examples/xstate/tailwind.config.jspackages/ignite-element/src/examples/xstate/taskManagerMachine.tspackages/ignite-element/src/examples/xstate/tsconfig.jsonpackages/ignite-element/src/examples/xstate/vite.config.tspackages/ignite-element/src/examples/xstate/xstateCounterMachine.tspackages/ignite-element/src/examples/xstate/xstateExample.tsxpackages/ignite-element/src/examples/xstate/xstateTaskManager.tspackages/ignite-element/src/globalStyles.tspackages/ignite-element/src/igniteCore/index.tspackages/ignite-element/src/igniteCore/mobxEntry.tspackages/ignite-element/src/igniteCore/types.tspackages/ignite-element/src/igniteCore/xstateEntry.tspackages/ignite-element/src/index.tspackages/ignite-element/src/injectStyles.tspackages/ignite-element/src/internal/setupDomPolyfill.tspackages/ignite-element/src/jsx/index.tspackages/ignite-element/src/jsx/jsx-dev-runtime.tspackages/ignite-element/src/jsx/jsx-runtime.tspackages/ignite-element/src/mobx.tspackages/ignite-element/src/plugins/configFile.tspackages/ignite-element/src/plugins/viteIgniteConfigPlugin.tspackages/ignite-element/src/plugins/webpackIgniteConfigPlugin.tspackages/ignite-element/src/redux.tspackages/ignite-element/src/renderers/LitRenderStrategy.tspackages/ignite-element/src/renderers/RenderStrategy.tspackages/ignite-element/src/renderers/ignite-jsx.tspackages/ignite-element/src/renderers/jsx/IgniteJsxRenderStrategy.tspackages/ignite-element/src/renderers/jsx/index.tspackages/ignite-element/src/renderers/jsx/jsx-dev-runtime.tspackages/ignite-element/src/renderers/jsx/jsx-runtime.tspackages/ignite-element/src/renderers/jsx/jsx.d.tspackages/ignite-element/src/renderers/jsx/noDiffDenylist.tspackages/ignite-element/src/renderers/jsx/renderer.tspackages/ignite-element/src/renderers/jsx/types.tspackages/ignite-element/src/renderers/lit.tspackages/ignite-element/src/renderers/resolveConfiguredRenderStrategy.tspackages/ignite-element/src/runtime/agent.tspackages/ignite-element/src/runtime/schema.tspackages/ignite-element/src/testing.tspackages/ignite-element/src/tests/IgniteCore.test.tspackages/ignite-element/src/tests/IgniteElement.test.tsxpackages/ignite-element/src/tests/IgniteElementFactory.test.tspackages/ignite-element/src/tests/IgniteRedux.test.tsxpackages/ignite-element/src/tests/MockAdapter.tspackages/ignite-element/src/tests/adapters/MobxAdapter.test.tspackages/ignite-element/src/tests/adapters/ReduxAdapter.test.tspackages/ignite-element/src/tests/adapters/XStateAdapter.test.tspackages/ignite-element/src/tests/config.test.tspackages/ignite-element/src/tests/configFile.test.tspackages/ignite-element/src/tests/createComponentFactory.test.tspackages/ignite-element/src/tests/globalStyles.test.tspackages/ignite-element/src/tests/helpers/vitePluginHarness.test.tspackages/ignite-element/src/tests/helpers/vitePluginHarness.tspackages/ignite-element/src/tests/injectStyles.test.tspackages/ignite-element/src/tests/internal/setupDomPolyfill.test.tspackages/ignite-element/src/tests/plugins/igniteConfigPlugins.test.tspackages/ignite-element/src/tests/renderers/diffing.behavior.test.tspackages/ignite-element/src/tests/renderers/igniteJsxRenderStrategy.test.tspackages/ignite-element/src/tests/renderers/jsxRuntime.test.tspackages/ignite-element/src/tests/renderers/litRenderStrategy.test.tspackages/ignite-element/src/tests/renderers/renderer.behavior.test.tspackages/ignite-element/src/tests/renderers/renderer.test.tspackages/ignite-element/src/tests/renderers/resolveConfiguredRenderStrategy.test.tspackages/ignite-element/src/tests/setup/registerRenderersForTests.tspackages/ignite-element/src/tests/testing.test.tspackages/ignite-element/src/tests/tsconfig.jsonpackages/ignite-element/src/tests/types/igniteCore.types.test.tspackages/ignite-element/src/tests/types/testing.types.test.tspackages/ignite-element/src/types/agent.tspackages/ignite-element/src/types/schema.tspackages/ignite-element/src/utils/__tests__/adapterGuards.test.tspackages/ignite-element/src/utils/__tests__/matchState.test.tspackages/ignite-element/src/utils/adapterGuards.tspackages/ignite-element/src/utils/igniteRedux.tspackages/ignite-element/src/utils/matchState.tspackages/ignite-element/src/utils/mobxGuards.tspackages/ignite-element/src/vite-env.d.tspackages/ignite-element/src/xstate.tspackages/ignite-element/tsconfig.examples.jsonpackages/ignite-element/tsconfig.jsonpackages/ignite-element/tsconfig.typecheck.jsonpackages/ignite-element/vite.config.tspackages/ignite-element/vitest.config.tspackages/ignite-element/vitest.node.config.tspackages/ignite-element/vitest.setup.tspackages/ignite-renderer/README.mdpackages/ignite-renderer/package.jsonpackages/ignite-renderer/src/config.tspackages/ignite-renderer/src/globalStyles.tspackages/ignite-renderer/src/index.tspackages/ignite-renderer/src/injectStyles.tspackages/ignite-renderer/src/internal/setupDomPolyfill.tspackages/ignite-renderer/src/jsx/index.tspackages/ignite-renderer/src/jsx/jsx-dev-runtime.tspackages/ignite-renderer/src/jsx/jsx-runtime.tspackages/ignite-renderer/src/renderers/LitRenderStrategy.tspackages/ignite-renderer/src/renderers/RenderStrategy.tspackages/ignite-renderer/src/renderers/ignite-jsx.tspackages/ignite-renderer/src/renderers/jsx/IgniteJsxRenderStrategy.tspackages/ignite-renderer/src/renderers/jsx/index.tspackages/ignite-renderer/src/renderers/jsx/jsx-dev-runtime.tspackages/ignite-renderer/src/renderers/jsx/jsx-runtime.tspackages/ignite-renderer/src/renderers/jsx/noDiffDenylist.tspackages/ignite-renderer/src/renderers/jsx/renderer.tspackages/ignite-renderer/src/renderers/jsx/types.tspackages/ignite-renderer/src/renderers/lit.tspackages/ignite-renderer/src/renderers/registry.tspackages/ignite-renderer/tsconfig.jsonpackages/ignite-renderer/vite.config.tspnpm-workspace.yamlscripts/migrate-emit-to-effects.mjssrc/createComponentFactory.tssrc/examples/mobx/vite.config.tssrc/examples/xstate/ignite.config.tssrc/examples/xstate/index.htmlsrc/examples/xstate/tsconfig.jsonsrc/examples/xstate/vite.config.tssrc/igniteCore/redux.tssrc/igniteCore/types.tssrc/index.tssrc/mobx.tssrc/redux.tssrc/xstate.tstsconfig.jsonvite.config.tsvitest.config.ts
💤 Files with no reviewable changes (14)
- src/examples/xstate/tsconfig.json
- tsconfig.json
- src/examples/mobx/vite.config.ts
- src/examples/xstate/index.html
- src/examples/xstate/ignite.config.ts
- src/mobx.ts
- src/xstate.ts
- vite.config.ts
- src/examples/xstate/vite.config.ts
- src/igniteCore/redux.ts
- src/index.ts
- src/igniteCore/types.ts
- src/redux.ts
- src/createComponentFactory.ts
| "behaviorBoundaries": { | ||
| "functionalCore": [ | ||
| "packages/ignite-core/src/events.ts", | ||
| "packages/ignite-core/src/RenderArgs.ts", | ||
| "packages/ignite-core/src/utils/", | ||
| "packages/ignite-core/src/utils/matchState.ts", | ||
| "packages/ignite-element/src/types/" | ||
| ], | ||
| "shell": [ | ||
| "packages/ignite-core/src/createProjectionFactory.ts", | ||
| "packages/ignite-core/src/runtime/", | ||
| "packages/ignite-core/src/types.ts", | ||
| "packages/ignite-adapters/src/", | ||
| "packages/ignite-store/src/", | ||
| "packages/ignite-adapters/src/igniteCore/", | ||
| "packages/ignite-element/src/runtime/", | ||
| "packages/ignite-element/src/testing.ts", | ||
| "packages/ignite-element/src/IgniteCore.ts", | ||
| "packages/ignite-element/src/IgniteElementFactory.ts", | ||
| "packages/ignite-element/src/createComponentFactory.ts", | ||
| "packages/ignite-element/src/index.ts", | ||
| "packages/ignite-element/src/xstate.ts", | ||
| "packages/ignite-element/src/redux.ts", | ||
| "packages/ignite-element/src/mobx.ts" | ||
| ], | ||
| "adapters": [ | ||
| "packages/ignite-renderer/src/config.ts", | ||
| "packages/ignite-element/src/config/loadIgniteConfig.ts", | ||
| "packages/ignite-element/src/plugins/viteIgniteConfigPlugin.ts", | ||
| "packages/ignite-element/src/plugins/webpackIgniteConfigPlugin.ts" | ||
| ], | ||
| "lifecycle": [], | ||
| "scope": "changed-only", | ||
| "baseBranch": "main" | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Validating behavior boundary paths from .fas-config.json"
jq -r '
.behaviorBoundaries.functionalCore[],
.behaviorBoundaries.shell[],
.behaviorBoundaries.adapters[]
' .fas-config.json | while IFS= read -r p; do
if [ -e "$p" ]; then
echo "OK $p"
else
echo "MISSING $p"
fi
doneRepository: 0xjcf/ignite-element
Length of output: 1309
Remove packages/ignite-store/src/ from the shell boundary list—this path does not exist.
The validation found that packages/ignite-store/src/ is missing from the repository. This entry in the shell boundaries (line 34) will cause FAS enforcement to silently skip coverage for a configured boundary, weakening drift detection without obvious failure. Either remove this path from the configuration or verify whether it was restructured to a different location during the monorepo reorganization.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.fas-config.json around lines 23 - 57, The shell boundary in
behaviorBoundaries contains a non-existent path "packages/ignite-store/src/";
remove that entry (or replace it with the correct new path if the store was
moved) from the "shell" array in .fas-config.json so the "behaviorBoundaries" ->
"shell" list no longer references "packages/ignite-store/src/" (look for the
"behaviorBoundaries" object and the "shell" array to locate the entry).
| import { igniteCore } from "ignite-element"; // DOM binding over ignite-core | ||
|
|
||
| const socketComponent = igniteCore({ | ||
| source: socketActor, |
There was a problem hiding this comment.
igniteCore import path appears inconsistent with current package surface.
Line 198 imports igniteCore from ignite-element, but the package changelog indicates root entry no longer exports igniteCore. This example can send users to a broken import path.
📝 Suggested doc correction
-import { igniteCore } from "ignite-element"; // DOM binding over ignite-core
+import { igniteCore } from "ignite-core";If this example is intended to use a binding-specific entrypoint, switch to that explicit entrypoint instead and keep it consistent across all snippets in this doc.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { igniteCore } from "ignite-element"; // DOM binding over ignite-core | |
| const socketComponent = igniteCore({ | |
| source: socketActor, | |
| import { igniteCore } from "ignite-core"; | |
| const socketComponent = igniteCore({ | |
| source: socketActor, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/ignite-websocket.md` around lines 198 - 201, The import path for
igniteCore is incorrect: update the example to import igniteCore from the
package entry that actually exports it (use the binding-specific entrypoint
instead of "ignite-element" root), e.g., change the import used for igniteCore
so the example compiles and keep the same updated import across all snippets
that reference igniteCore, socketComponent, and socketActor to ensure
consistency.
| ```ts | ||
| const registerToggle = igniteCore({ | ||
| source: machine, | ||
| events: (event) => ({ | ||
| toggled: event<{ isOn: boolean }>(), | ||
| }), | ||
| commands: ({ actor }) => ({ | ||
| toggle: () => { | ||
| actor.send({ type: "TOGGLE" }); | ||
| }, | ||
| }), | ||
| }); | ||
| ``` |
There was a problem hiding this comment.
“Before” example doesn’t show the deprecated pattern.
The migration guide’s before-state should include emit inside commands; otherwise the migration delta is unclear.
Suggested doc fix
- commands: ({ actor }) => ({
+ commands: ({ actor, emit }) => ({
toggle: () => {
actor.send({ type: "TOGGLE" });
+ emit("toggled", { isOn: actor.getSnapshot().matches("on") });
},
}),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/site/src/content/docs/migration/effects-events.mdx` around lines 13 -
25, The "before" example for registerToggle using igniteCore doesn't demonstrate
the deprecated pattern because the commands block omits the use of emit; update
the example so that the commands: ({ actor }) => ({ toggle: () => { ... } })
includes the deprecated emit call (e.g., using actor.emit or emit(...) as shown
elsewhere in docs) so the migration delta clearly shows switching from emit to
actor.send; locate the registerToggle example, inside the igniteCore call, and
add the emit usage within the toggle command alongside or instead of actor.send
to reflect the pre-migration pattern.
| "peerDependencies": { | ||
| "@reduxjs/toolkit": ">=2.3.0", | ||
| "mobx": ">=6.13.5", | ||
| "redux": ">=5.0.1", | ||
| "xstate": ">=5.19.0" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect whether the root ignite-adapters entry eagerly imports every adapter ecosystem.
jq '.peerDependencies, .peerDependenciesMeta' packages/ignite-adapters/package.json
echo
fd '^(index|xstate|redux|mobx)\.ts$' packages/ignite-adapters/src -x sh -c '
echo "---- $1"
sed -n "1,160p" "$1"
' sh {}
echo
rg -n -C2 'from "(xstate|mobx|redux|@reduxjs/toolkit)"' packages/ignite-adapters/srcRepository: 0xjcf/ignite-element
Length of output: 13044
🏁 Script executed:
cat packages/ignite-adapters/package.jsonRepository: 0xjcf/ignite-element
Length of output: 1816
🏁 Script executed:
cat packages/ignite-adapters/src/index.tsRepository: 0xjcf/ignite-element
Length of output: 719
Mark peer dependencies as optional to match the modular adapter design.
The package exports separate entry points (./xstate, ./redux, ./mobx) and the root index.ts exports only MobX and Redux adapters—XState is not exported from the root at all. Requiring all four peers forces consumers using only one adapter (especially XState-only users importing via subpaths) to install unused ecosystems, which contradicts the one-adapter install intent and will surface as peer warnings/errors in stricter package managers.
📦 Add peerDependenciesMeta
"peerDependencies": {
"@reduxjs/toolkit": ">=2.3.0",
"mobx": ">=6.13.5",
"redux": ">=5.0.1",
"xstate": ">=5.19.0"
- }
+ },
+ "peerDependenciesMeta": {
+ "@reduxjs/toolkit": { "optional": true },
+ "mobx": { "optional": true },
+ "redux": { "optional": true },
+ "xstate": { "optional": true }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "peerDependencies": { | |
| "@reduxjs/toolkit": ">=2.3.0", | |
| "mobx": ">=6.13.5", | |
| "redux": ">=5.0.1", | |
| "xstate": ">=5.19.0" | |
| "peerDependencies": { | |
| "@reduxjs/toolkit": ">=2.3.0", | |
| "mobx": ">=6.13.5", | |
| "redux": ">=5.0.1", | |
| "xstate": ">=5.19.0" | |
| }, | |
| "peerDependenciesMeta": { | |
| "@reduxjs/toolkit": { "optional": true }, | |
| "mobx": { "optional": true }, | |
| "redux": { "optional": true }, | |
| "xstate": { "optional": true } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ignite-adapters/package.json` around lines 69 - 73, The
peerDependencies currently require all four libs; make them optional by adding a
peerDependenciesMeta object in packages/ignite-adapters/package.json that lists
each peer (`@reduxjs/toolkit`, mobx, redux, xstate) with "optional": true so
consumers can install only the adapter they use; keep the existing version
ranges unchanged and ensure the JSON key is added at the top-level alongside
peerDependencies.
| external: ["ignite-core", "@reduxjs/toolkit", "redux", "mobx", "xstate"], | ||
| globals: { | ||
| "@reduxjs/toolkit": "RTK", | ||
| redux: "Redux", | ||
| mobx: "MobX", | ||
| xstate: "XState", | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect core and adapters lib config for external/globals parity.
sed -n '1,220p' packages/ignite-core/vite.config.ts
sed -n '1,220p' packages/ignite-adapters/vite.config.ts
rg -nP --type=ts -C3 'external:\s*\['
rg -nP --type=ts -C5 'globals:\s*\{'Repository: 0xjcf/ignite-element
Length of output: 4251
🏁 Script executed:
# Find the createLibConfig implementation
find . -name "*.ts" -o -name "*.js" | xargs grep -l "createLibConfig" | head -5Repository: 0xjcf/ignite-element
Length of output: 248
🏁 Script executed:
# Check the lib config implementation
fd -type f -name "lib.ts" -o -name "lib.js" | grep -i config | head -5Repository: 0xjcf/ignite-element
Length of output: 234
🏁 Script executed:
# Look for any globals mapping documentation or package.json exports
rg -l "globals" --type=ts --type=json | head -10Repository: 0xjcf/ignite-element
Length of output: 248
🏁 Script executed:
# Search for IgniteCore or ignite-core global references
rg "IgniteCore|ignite-core.*global" --type=ts --type=json -iRepository: 0xjcf/ignite-element
Length of output: 15612
🏁 Script executed:
cat configs/vite/lib.tsRepository: 0xjcf/ignite-element
Length of output: 810
🏁 Script executed:
# Check ignite-core's output/exports configuration
cat packages/ignite-core/package.json | head -50Repository: 0xjcf/ignite-element
Length of output: 881
🏁 Script executed:
# Look for any build output or dist references that show the global name
rg "IgniteCore|ignite-core" configs/vite/ -A5 -B5Repository: 0xjcf/ignite-element
Length of output: 46
🏁 Script executed:
sed -n '34,50p' packages/ignite-element/vite.config.tsRepository: 0xjcf/ignite-element
Length of output: 274
Add a global mapping for ignite-core external.
Line 13 marks ignite-core as external, but the globals object (lines 14–19) does not provide its global name. For UMD consumers, this causes improper bindings and can result in runtime errors or undefined references.
Suggested fix
globals: {
+ "ignite-core": "IgniteCore",
"@reduxjs/toolkit": "RTK",
redux: "Redux",
mobx: "MobX",
xstate: "XState",
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| external: ["ignite-core", "@reduxjs/toolkit", "redux", "mobx", "xstate"], | |
| globals: { | |
| "@reduxjs/toolkit": "RTK", | |
| redux: "Redux", | |
| mobx: "MobX", | |
| xstate: "XState", | |
| }, | |
| external: ["ignite-core", "@reduxjs/toolkit", "redux", "mobx", "xstate"], | |
| globals: { | |
| "ignite-core": "IgniteCore", | |
| "@reduxjs/toolkit": "RTK", | |
| redux: "Redux", | |
| mobx: "MobX", | |
| xstate: "XState", | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ignite-adapters/vite.config.ts` around lines 13 - 19, The externals
list includes "ignite-core" but the globals mapping in the Vite/Rollup config
(the object next to external in vite.config.ts) lacks a corresponding entry; add
a globals key for "ignite-core" (e.g., "ignite-core": "IgniteCore") to the
globals object to ensure UMD consumers resolve the external correctly and avoid
undefined references when building via the Rollup/Vite bundle.
| when<CommandName extends keyof Commands & string>( | ||
| commandName: CommandName, | ||
| payload?: unknown, | ||
| ): IgniteTestScenario<State, Commands, Events>; |
There was a problem hiding this comment.
Keep when() aligned with the command signature.
The DSL drops the command payload type here by making it unknown and optional, so required payloads and wrong payload types both compile and only fail once execute() runs. That undercuts the typed testing surface this file is introducing.
🛠️ Suggested typing fix
export type IgniteTestScenario<
State,
Commands extends FacadeCommandResult = FacadeCommandResult,
Events extends EventMap = EmptyEventMap,
> = {
given(
expected: IgniteStateExpectation<State>,
): IgniteTestScenario<State, Commands, Events>;
when<CommandName extends keyof Commands & string>(
commandName: CommandName,
- payload?: unknown,
+ ...args: Parameters<Commands[CommandName]> extends []
+ ? []
+ : [Parameters<Commands[CommandName]>[0]]
): IgniteTestScenario<State, Commands, Events>;
@@
when<CommandName extends keyof Commands & string>(
commandName: CommandName,
- payload?: unknown,
+ ...args: Parameters<Commands[CommandName]> extends []
+ ? []
+ : [Parameters<Commands[CommandName]>[0]]
) {
this.lastResult = this.component.execute(
commandName,
- payload as Parameters<Commands[CommandName]>[0],
+ args[0] as Parameters<Commands[CommandName]>[0],
);
return this;
}If execute() is meant to support the full command signature, forward ...Parameters<Commands[CommandName]> instead of only the first slot.
Also applies to: 212-220
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ignite-element/src/testing.ts` around lines 49 - 52, The when()
overload currently accepts payload?: unknown which drops command payload types;
change its signature to forward the real command parameters using
...Parameters<Commands[CommandName]> so required and typed payloads from the
Commands map are preserved; update any equivalent overloads/occurrences
(including the execute() usage and other when() declarations) to accept the same
parameter tuple and propagate those types into IgniteTestScenario<State,
Commands, Events> so calls to when(commandName, ...) are fully type-checked
against Commands[CommandName].
| const matchedEvent = events.find((event) => event.type === type); | ||
|
|
||
| if (!matchedEvent) { | ||
| throw new Error( | ||
| `[igniteTest] Expected event "${type}" but received ${formatValue(events)}.`, | ||
| ); | ||
| } | ||
|
|
||
| if (typeof payload === "undefined") { | ||
| return; | ||
| } | ||
|
|
||
| if (!valuesMatch(matchedEvent.payload, payload)) { | ||
| throw new Error( | ||
| `[igniteTest] Event "${type}" payload mismatch.\nExpected: ${formatValue(payload)}\nReceived: ${formatValue(matchedEvent.payload)}`, |
There was a problem hiding this comment.
Match the payload before picking an emitted event.
This helper grabs the first event with a matching type and only checks the payload afterwards. If the runtime emits the same type more than once, expectEvent() can fail against the first payload even when a later emission matches, and expectEvents() inherits the same behavior.
🛠️ Suggested matching fix
- const matchedEvent = events.find((event) => event.type === type);
-
- if (!matchedEvent) {
+ const matchedEvents = events.filter((event) => event.type === type);
+
+ if (matchedEvents.length === 0) {
throw new Error(
`[igniteTest] Expected event "${type}" but received ${formatValue(events)}.`,
);
}
if (typeof payload === "undefined") {
return;
}
- if (!valuesMatch(matchedEvent.payload, payload)) {
+ const matchedEvent = matchedEvents.find((event) =>
+ valuesMatch(event.payload, payload),
+ );
+
+ if (!matchedEvent) {
throw new Error(
- `[igniteTest] Event "${type}" payload mismatch.\nExpected: ${formatValue(payload)}\nReceived: ${formatValue(matchedEvent.payload)}`,
+ `[igniteTest] Event "${type}" payload mismatch.\nExpected: ${formatValue(payload)}\nReceived: ${formatValue(matchedEvents.map((event) => event.payload))}`,
);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ignite-element/src/testing.ts` around lines 176 - 190, The helper
currently finds the first event by type and only then checks payload, which
fails if multiple events share type; update the search to match type and payload
up front: in expectEvent (and similarly in expectEvents) replace the existing
find((event) => event.type === type) with a predicate that also checks payload
(e.g., find((event) => event.type === type && (typeof payload === "undefined" ||
valuesMatch(event.payload, payload)))), then remove or simplify the subsequent
payload comparison so the function throws a single clear error when no matching
event (type+payload) is found; keep the existing error messages/formatValue
usage for consistency.
| external: [ | ||
| "ignite-core", | ||
| "ignite-renderer", | ||
| "ignite-adapters", | ||
| "lit-html", | ||
| "xstate", | ||
| "mobx", | ||
| "redux", | ||
| "@reduxjs/toolkit", | ||
| "node:fs", | ||
| "node:path", | ||
| "node:url", | ||
| ], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP -t ts -t tsx \
-g 'packages/ignite-element/src/**' \
-g '!**/*.test.*' \
-g '!**/__tests__/**' \
-g '!**/tests/**' \
'from\s+"(ignite-(core|renderer|adapters)(/[^"]+)?|xstate(/[^"]+)?)"|import\(\s*"(ignite-(core|renderer|adapters)(/[^"]+)?|xstate(/[^"]+)?)"\s*\)'Repository: 0xjcf/ignite-element
Length of output: 91
🏁 Script executed:
# First, explore the structure of packages/ignite-element/src
fd -t f 'src' packages/ignite-element/ | head -20Repository: 0xjcf/ignite-element
Length of output: 46
🏁 Script executed:
# Check what rg file types are available
rg --type-list | grep -i 'ts\|javascript'Repository: 0xjcf/ignite-element
Length of output: 755
🏁 Script executed:
# Search for actual subpath imports with corrected approach
rg -n 'from\s+["\x27](ignite-core|ignite-renderer|ignite-adapters|xstate)' packages/ignite-element/src/ --max-count=50Repository: 0xjcf/ignite-element
Length of output: 7828
Use regex patterns to externalize subpath imports.
The source code imports from subpaths like ignite-renderer/jsx, ignite-renderer/lit, ignite-renderer/jsx-runtime, ignite-renderer/jsx-dev-runtime, ignite-adapters/xstate, and ignite-adapters/mobx. Rollup only treats string external entries as exact module ids—bare strings like "ignite-renderer" do not match ignite-renderer/jsx. These subpath imports will be bundled into ignite-element instead of staying external.
🔧 Proposed fix
external: [
- "ignite-core",
- "ignite-renderer",
- "ignite-adapters",
+ /^ignite-core(?:\/.*)?$/,
+ /^ignite-renderer(?:\/.*)?$/,
+ /^ignite-adapters(?:\/.*)?$/,
"lit-html",
- "xstate",
+ /^xstate(?:\/.*)?$/,
"mobx",
"redux",
"@reduxjs/toolkit",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| external: [ | |
| "ignite-core", | |
| "ignite-renderer", | |
| "ignite-adapters", | |
| "lit-html", | |
| "xstate", | |
| "mobx", | |
| "redux", | |
| "@reduxjs/toolkit", | |
| "node:fs", | |
| "node:path", | |
| "node:url", | |
| ], | |
| external: [ | |
| /^ignite-core(?:\/.*)?$/, | |
| /^ignite-renderer(?:\/.*)?$/, | |
| /^ignite-adapters(?:\/.*)?$/, | |
| "lit-html", | |
| /^xstate(?:\/.*)?$/, | |
| "mobx", | |
| "redux", | |
| "@reduxjs/toolkit", | |
| "node:fs", | |
| "node:path", | |
| "node:url", | |
| ], |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ignite-element/vite.config.ts` around lines 21 - 33, The external
array in vite.config.ts currently lists bare module ids (e.g.,
"ignite-renderer", "ignite-adapters") which won’t match subpath imports like
"ignite-renderer/jsx" or "ignite-adapters/mobx"; update the external
configuration to use regex patterns that match the package and any subpaths
(e.g., patterns for ignite-renderer, ignite-adapters, ignite-core, lit-html,
xstate, mobx, redux, `@reduxjs/toolkit`, and node:fs/path/url) so Rollup treats
those subpath imports as external rather than bundling them into ignite-element.
| alias: [ | ||
| { | ||
| find: "ignite-element", | ||
| replacement: resolvePath("./src/index.ts"), | ||
| }, | ||
| { | ||
| find: "ignite-element/", | ||
| replacement: resolvePath("./src/"), | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In current Vite/Rollup alias behavior, when find is a string (e.g. "ignite-element"), does it also match subpath imports like "ignite-element/xstate"? What is the recommended configuration to distinguish exact package aliases from subpath aliases?
💡 Result:
Yes. Vite’s resolve.alias is passed through to @rollup/plugin-alias’s entries option [2], and that plugin treats a string find as matching either:
- the exact import (
importee === find), or - any subpath import starting with
find + '/'(i.e.importee.startsWith(find + '/')) [1].
So find: "ignite-element" will also match ignite-element/xstate [1].
Recommended config to distinguish “exact package” vs “subpath”
Use RegExp aliases (and rely on ordering: first match wins [1]):
// vite.config.ts
export default {
resolve: {
alias: [
// exact package import: import 'ignite-element'
{ find: /^ignite-element$/, replacement: '/path/to/ignite-element/index.ts' },
// subpath imports: import 'ignite-element/xstate'
{ find: /^ignite-element\/(.+)$/, replacement: '/path/to/ignite-element/$1' },
],
},
}This is also the approach Vite recommends when you need “more precise control” over alias matching [3].
Sources: [1] [2] [3]
Use RegExp aliases to prevent ambiguous import resolution.
The current string-based find patterns create unintended alias collisions. find: "ignite-element" matches both the exact import and any subpath imports (e.g., ignite-element/xstate), making the second alias redundant and ambiguous. Vite's resolve.alias evaluates first match wins, so these overlapping patterns can cause unexpected behavior.
Use RegExp patterns for precise control:
🔧 Proposed fix
{
- find: "ignite-element",
- replacement: resolvePath("./src/index.ts"),
- },
- {
- find: "ignite-element/",
- replacement: resolvePath("./src/"),
+ find: /^ignite-element$/,
+ replacement: resolvePath("./src/index.ts"),
+ },
+ {
+ find: /^ignite-element\/(.+)$/,
+ replacement: resolvePath("./src/$1"),
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| alias: [ | |
| { | |
| find: "ignite-element", | |
| replacement: resolvePath("./src/index.ts"), | |
| }, | |
| { | |
| find: "ignite-element/", | |
| replacement: resolvePath("./src/"), | |
| }, | |
| alias: [ | |
| { | |
| find: /^ignite-element$/, | |
| replacement: resolvePath("./src/index.ts"), | |
| }, | |
| { | |
| find: /^ignite-element\/(.+)$/, | |
| replacement: resolvePath("./src/$1"), | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ignite-element/vitest.config.ts` around lines 16 - 24, The alias
entries in the Vite config are overlapping because the string-based find
"ignite-element" also matches subpath imports, so update the alias array used by
vitest/vite (the alias object entries created near alias: [...] referencing
resolvePath("./src/index.ts") and resolvePath("./src/")) to use RegExp find
patterns that precisely match the exact package and subpaths (e.g., a RegExp for
the exact package name and a separate RegExp for subpaths) so the resolver does
not treat the first entry as a catch-all; ensure the first alias targets the
exact package import and the second targets all subpaths to preserve intended
behavior.
| const fallback = registry.get("ignite-jsx") ?? registry.values().next().value; | ||
| if (!fallback) { | ||
| throw new Error( | ||
| '[ignite-element] No render strategies have been registered. Import "ignite-element/renderers/ignite-jsx" (or another strategy entry point) before registering components.', | ||
| '[ignite-renderer] No render strategies have been registered. Import "ignite-renderer/jsx" (or another strategy entry point) before registering components.', | ||
| ); | ||
| } | ||
|
|
||
| if (renderer !== "ignite-jsx") { | ||
| console.warn( | ||
| `[ignite-element] Render strategy "${renderer}" is not registered. Ensure your config loader imports "ignite-element/renderers/${renderer}" (the official plugins handle this automatically). Falling back to Ignite JSX.`, | ||
| `[ignite-renderer] Render strategy "${renderer}" is not registered. Ensure you import "ignite-renderer/${renderer}" before registering components. Falling back to Ignite JSX.`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Missing warning when "ignite-jsx" is requested but unavailable can hide misconfiguration.
On Line 46, warning is skipped for renderer === "ignite-jsx". If Ignite JSX is not registered and another strategy exists, resolution silently falls back to a different renderer.
🔧 Proposed fix
export function resolveRenderStrategy(
renderer: string,
): RenderStrategyFactory<unknown> {
const registry = getRegistry();
const registered = registry.get(renderer);
if (registered) {
return registered;
}
- const fallback = registry.get("ignite-jsx") ?? registry.values().next().value;
- if (!fallback) {
+ const igniteJsx = registry.get("ignite-jsx");
+ const firstEntry = registry.entries().next().value as
+ | [string, RenderStrategyFactory<unknown>]
+ | undefined;
+ const fallbackEntry = igniteJsx
+ ? (["ignite-jsx", igniteJsx] as const)
+ : firstEntry;
+ if (!fallbackEntry) {
throw new Error(
'[ignite-renderer] No render strategies have been registered. Import "ignite-renderer/jsx" (or another strategy entry point) before registering components.',
);
}
+ const [fallbackName, fallback] = fallbackEntry;
- if (renderer !== "ignite-jsx") {
+ if (renderer !== fallbackName) {
console.warn(
- `[ignite-renderer] Render strategy "${renderer}" is not registered. Ensure you import "ignite-renderer/${renderer}" before registering components. Falling back to Ignite JSX.`,
+ `[ignite-renderer] Render strategy "${renderer}" is not registered. Ensure you import "ignite-renderer/${renderer}" before registering components. Falling back to "${fallbackName}".`,
);
}
return fallback;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ignite-renderer/src/renderers/registry.ts` around lines 39 - 50, The
current fallback logic in the registry resolution uses const fallback =
registry.get("ignite-jsx") ?? registry.values().next().value and skips the
warning when renderer === "ignite-jsx", which hides misconfiguration if
"ignite-jsx" isn't registered; update the code that checks the requested
renderer (the renderer variable) to also emit a console.warn when renderer ===
"ignite-jsx" but registry.get("ignite-jsx") is falsy and fallback points to a
different strategy, using the same warning style and message as the other branch
so callers see that the requested "ignite-jsx" entry wasn't present and a
different renderer was chosen (refer to registry.get("ignite-jsx"), fallback,
and renderer to locate and change the logic).
Ignore local Codex config state. Preserve updated FAS/Codex orchestration guidance. Format the curated domain map. Verification: .fas/scripts/verify.sh
…tching - Unify effects callback to single object-arg pattern (FacadeEffectArgs) - Effects now run post-render via queueMicrotask (React useEffect semantics) - JSX renderer uses positional patching instead of full replacement on reorder - Add exposeCommands() to surface command functions on custom elements - Make execute/story.execute/until async to support post-render effects - Update IgniteTestScenario.when() to return Promise - Fix lint (noNonNullAssertion) and align all tests to new signatures
…mmands - Add inferObservedAttributes() to detect single-arg setX commands - Add processInitialAttributes() for attributes set before upgrade - Add setupAttributeObservation() using MutationObserver for dynamic changes - Convention: command setX(value) with .length === 1 → attribute x - Zero new API surface — developers write commands exactly as before - Eliminates race condition in parent-child component coordination
Changes
Verification Results
Safety
Summary by CodeRabbit
Release Notes
New Features
effectscallback for state-driven DOM event emission, movingemitfromcommands.statestoviewfor clearer state projection semantics.execute,getState,getView,getSchema,on,watch).Documentation
ignite.config.tsor bundler plugin for default setup.Improvements