diff --git a/PR_CHANGES.md b/PR_CHANGES.md new file mode 100644 index 0000000000..3abc6ea2d0 --- /dev/null +++ b/PR_CHANGES.md @@ -0,0 +1,71 @@ +# Fix: Add runtime warning for unknown delay keys in `after` transitions + +## Problem + +According to the XState documentation, delays should be typed when defined in `setup()`. However, due to a TypeScript limitation (issue #55709), the compiler cannot properly validate delay keys in `after` transitions when using `setup().createMachine()`. + +Additionally, when an unknown delay key is referenced at runtime, the machine silently treats it as an immediate transition (no delay), which can lead to unexpected behavior that's difficult to debug. + +## Solution + +### 1. Runtime Warning (Development Mode) + +Added a development-mode warning in `packages/core/src/actions/raise.ts` that alerts developers when they reference an unknown delay key: + +```typescript +if (isDevelopment && configDelay === undefined) { + console.warn( + `Delay "${delay}" is not configured in \`delays\`. The event will be raised immediately. ` + + `This is likely a mistake. Available delays: ${Object.keys(delaysMap || {}).join(', ') || 'none'}.` + ); +} +``` + +This warning helps developers catch delay configuration mistakes during development. + +### 2. Test Coverage + +Added comprehensive tests in `packages/core/test/delays.test.ts` that verify: +- Warning is shown when unknown delay is referenced +- Warning is shown when no delays are configured +- No warning for numeric delays +- No warning for properly configured delays +- Immediate transition behavior is documented + +### 3. Documentation + +Updated type test comments in `packages/core/test/setup.types.test.ts` to clarify that the TypeScript limitation is known and that runtime warnings provide the safety net. + +## TypeScript Limitation + +The `@x-ts-expect-error` annotations in the type tests reference TypeScript issue #55709, which prevents proper narrowing of mapped types with string unions. This is a compiler limitation, not an XState issue. + +However, XState DOES provide type checking when using `createMachine` directly with `types: { delays: ... }`: + +```typescript +createMachine({ + types: {} as { + delays: 'one second' | 'one minute'; + }, + after: { + // @ts-expect-error - properly caught! + 'unknown delay': {} + } +}); +``` + +## Testing + +Run the new tests: +```bash +pnpm -C packages/core test delays.test.ts +``` + +Run all tests: +```bash +pnpm test +``` + +## Breaking Changes + +None. This change only adds development-mode warnings and does not modify runtime behavior or public APIs. diff --git a/PR_DESCRIPTION.md b/PR_DESCRIPTION.md new file mode 100644 index 0000000000..631835a6f0 --- /dev/null +++ b/PR_DESCRIPTION.md @@ -0,0 +1,128 @@ +# Add development-mode warning for unknown delay keys in `after` transitions + +## Summary + +Fixes the issue where unknown delay keys in `after` transitions are silently treated as immediate transitions, making it difficult to debug configuration mistakes. + +## Problem + +When using `setup().createMachine()` with delays configured: + +```typescript +const machine = setup({ + delays: { + slotDuration: 100 + } +}).createMachine({ + states: { + sleep: { + after: { + slotDuration222: 'wake' // Typo! But no error shown + } + }, + wake: {} + } +}); +``` + +**Current behavior:** +1. TypeScript doesn't catch the typo due to a compiler limitation (TS#55709) +2. Runtime silently treats the unknown delay as immediate transition +3. Machine transitions instantly instead of waiting, causing hard-to-debug issues + +**Expected behavior:** +- Developers should be warned about the misconfiguration +- The issue should be caught during development + +## Solution + +### 1. Runtime Warning (Development Mode Only) + +Added a warning in `packages/core/src/actions/raise.ts` that triggers when an unknown delay key is referenced: + +```typescript +if (isDevelopment && configDelay === undefined) { + console.warn( + `Delay "${delay}" is not configured in \`delays\`. The event will be raised immediately. ` + + `This is likely a mistake. Available delays: ${Object.keys(delaysMap || {}).join(', ') || 'none'}.` + ); +} +``` + +**Example output:** +``` +Delay "slotDuration222" is not configured in `delays`. The event will be raised immediately. This is likely a mistake. Available delays: slotDuration. +``` + +### 2. Comprehensive Test Coverage + +Added `packages/core/test/delays.test.ts` with tests covering: +- ✅ Warning when unknown delay is referenced +- ✅ Warning when no delays are configured +- ✅ No warning for numeric delays +- ✅ No warning for properly configured delays +- ✅ Documented immediate transition behavior + +### 3. Documentation Updates + +Updated type test comments in `packages/core/test/setup.types.test.ts` to clarify: +- The TypeScript limitation (TS#55709) is known +- Runtime warnings provide the safety net +- The issue is a compiler limitation, not an XState bug + +## TypeScript Limitation Explained + +The `@x-ts-expect-error` annotations in type tests reference [TypeScript issue #55709](https://github.com/microsoft/TypeScript/issues/55709), which prevents proper narrowing of mapped types with string unions when used in `setup().createMachine()`. + +However, XState DOES provide type checking when using `createMachine` directly: + +```typescript +createMachine({ + types: {} as { + delays: 'one second' | 'one minute'; + }, + after: { + 'unknown delay': {} // @ts-expect-error - properly caught! + } +}); +``` + +## Changes + +### Modified Files +1. **packages/core/src/actions/raise.ts** - Added development-mode warning +2. **packages/core/test/setup.types.test.ts** - Updated comments to clarify known TypeScript limitation +3. **packages/core/test/delays.test.ts** - NEW: Added comprehensive runtime tests + +### Breaking Changes +None. This change: +- Only adds development-mode warnings (not in production) +- Does not modify runtime behavior +- Does not change public APIs +- Is fully backward compatible + +## Testing + +The new tests can be run with: +```bash +pnpm -C packages/core test delays.test.ts +``` + +All existing tests should still pass: +```bash +pnpm test +``` + +## Related Issues + +Addresses the reported issue where: +- TypeScript doesn't catch unknown delay keys in `after` transitions when using `setup()` +- Unknown delays cause immediate transitions without warning +- Developers have difficulty debugging delay configuration mistakes + +## Benefits + +1. **Better Developer Experience**: Clear warning messages help catch mistakes early +2. **Maintains Performance**: Warning only runs in development mode +3. **Backward Compatible**: Existing code continues to work without changes +4. **Documented**: Tests and comments clarify the TypeScript limitation diff --git a/SUMMARY.md b/SUMMARY.md new file mode 100644 index 0000000000..1f6e51be79 --- /dev/null +++ b/SUMMARY.md @@ -0,0 +1,89 @@ +# Summary of Changes for PR + +## Files Modified + +### 1. `packages/core/src/actions/raise.ts` +**Change**: Added development-mode warning when an unknown delay key is referenced + +**Code Added**: +```typescript +if (isDevelopment && configDelay === undefined) { + console.warn( + `Delay "${delay}" is not configured in \`delays\`. The event will be raised immediately. ` + + `This is likely a mistake. Available delays: ${Object.keys(delaysMap || {}).join(', ') || 'none'}.` + ); +} +``` + +### 2. `packages/core/test/setup.types.test.ts` +**Change**: Updated comments for two test cases to clarify the TypeScript limitation + +**Tests Updated**: +- "should not accept an after transition that references an unknown delay when delays are configured" +- "should not accept an after transition that references an unknown delay when delays are not configured" + +**Comment Added**: +```typescript +// @x-ts-expect-error https://github.com/microsoft/TypeScript/issues/55709 +// TypeScript limitation: mapped types with string unions don't narrow properly. +// A dev-mode runtime warning will be shown for this. +``` + +### 3. `packages/core/test/delays.test.ts` (NEW FILE) +**Change**: Created comprehensive runtime tests for delay behavior + +**Tests Added**: +- ✅ Warns when unknown delay is referenced (with configured delays) +- ✅ Warns when unknown delay is referenced (without configured delays) +- ✅ No warning for numeric delays +- ✅ No warning for properly configured delays +- ✅ Documents immediate transition behavior for unknown delays + +## Key Points + +### Problem Solved +- Users couldn't catch typos in delay keys at compile time (TypeScript limitation) +- Unknown delays caused immediate transitions without any warning +- Difficult to debug delay configuration mistakes + +### Solution Approach +- Added development-mode runtime warning (no production overhead) +- Comprehensive test coverage +- Documentation of TypeScript limitation +- Fully backward compatible + +### TypeScript Limitation +The issue is due to TypeScript #55709 - mapped types with string unions don't properly narrow in certain contexts. This affects `setup().createMachine()` but NOT `createMachine()` with `types: { delays: ... }`. + +## How to Use This PR + +### For PR Description +Use the content from `/workspaces/xstate/PR_DESCRIPTION.md` + +### For Commit Message +``` +fix(core): add dev-mode warning for unknown delay keys in after transitions + +Adds a development-mode warning when an unknown delay key is referenced +in `after` transitions. This helps developers catch configuration mistakes +that TypeScript cannot detect due to a compiler limitation (TS#55709). + +The warning only runs in development mode and does not affect production. + +Fixes: [Issue Number] +``` + +### Testing Instructions +```bash +# Run new delay tests +pnpm -C packages/core test delays.test.ts + +# Run all tests +pnpm test +``` + +## Benefits +1. **Better DX**: Clear warnings help catch mistakes early +2. **Zero Runtime Cost**: Only runs in development mode +3. **Backward Compatible**: No breaking changes +4. **Well Documented**: Tests and comments explain the TypeScript limitation diff --git a/packages/core/src/actions/raise.ts b/packages/core/src/actions/raise.ts index 5d8e699cdf..fdd328da64 100644 --- a/packages/core/src/actions/raise.ts +++ b/packages/core/src/actions/raise.ts @@ -66,6 +66,12 @@ function resolveRaise( let resolvedDelay: number | undefined; if (typeof delay === 'string') { const configDelay = delaysMap && delaysMap[delay]; + if (isDevelopment && configDelay === undefined) { + console.warn( + `Delay "${delay}" is not configured in \`delays\`. The event will be raised immediately. ` + + `This is likely a mistake. Available delays: ${Object.keys(delaysMap || {}).join(', ') || 'none'}.` + ); + } resolvedDelay = typeof configDelay === 'function' ? configDelay(args, actionParams) diff --git a/packages/core/test/delays.test.ts b/packages/core/test/delays.test.ts new file mode 100644 index 0000000000..225dbc3482 --- /dev/null +++ b/packages/core/test/delays.test.ts @@ -0,0 +1,145 @@ +import { createActor, setup } from '../src/index.ts'; + +describe('delays runtime behavior', () => { + let consoleWarnSpy: ReturnType; + + beforeEach(() => { + consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + + afterEach(() => { + consoleWarnSpy.mockRestore(); + }); + + it('should warn in development when referencing an unknown delay in after transition', () => { + const machine = setup({ + delays: { + knownDelay: 100 + } + }).createMachine({ + initial: 'a', + states: { + a: { + after: { + unknownDelay: 'b' as any // TypeScript can't catch this due to TS#55709 + } + }, + b: {} + } + }); + + const actor = createActor(machine); + actor.start(); + + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining('Delay "unknownDelay" is not configured') + ); + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining('Available delays: knownDelay') + ); + }); + + it('should warn when no delays are configured and a string delay is referenced', () => { + const machine = setup({}).createMachine({ + initial: 'a', + states: { + a: { + after: { + someDelay: 'b' as any // TypeScript can't catch this due to TS#55709 + } + }, + b: {} + } + }); + + const actor = createActor(machine); + actor.start(); + + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining('Delay "someDelay" is not configured') + ); + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining('Available delays: none') + ); + }); + + it('should not warn when using a numeric delay', () => { + const machine = setup({}).createMachine({ + initial: 'a', + states: { + a: { + after: { + 100: 'b' + } + }, + b: {} + } + }); + + const actor = createActor(machine); + actor.start(); + + expect(consoleWarnSpy).not.toHaveBeenCalled(); + }); + + it('should not warn when using a configured delay', () => { + const machine = setup({ + delays: { + myDelay: 100 + } + }).createMachine({ + initial: 'a', + states: { + a: { + after: { + myDelay: 'b' + } + }, + b: {} + } + }); + + const actor = createActor(machine); + actor.start(); + + expect(consoleWarnSpy).not.toHaveBeenCalled(); + }); + + it('should transition immediately when unknown delay is used (current behavior)', () => { + return new Promise((resolve, reject) => { + const machine = setup({ + delays: { + knownDelay: 100 + } + }).createMachine({ + initial: 'a', + states: { + a: { + after: { + unknownDelay: 'b' as any // TypeScript can't catch this due to TS#55709 + } + }, + b: { + type: 'final' + } + } + }); + + const actor = createActor(machine); + + actor.subscribe({ + complete: () => { + // Should complete immediately since unknownDelay is undefined + resolve(); + } + }); + + actor.start(); + + // If the test doesn't complete quickly, it means the transition didn't happen immediately + setTimeout(() => { + reject(new Error('Should have transitioned immediately')); + }, 50); + }); + }); +}); diff --git a/packages/core/test/setup.types.test.ts b/packages/core/test/setup.types.test.ts index b09c89f44a..5387e0d72b 100644 --- a/packages/core/test/setup.types.test.ts +++ b/packages/core/test/setup.types.test.ts @@ -2028,6 +2028,8 @@ describe('setup()', () => { a: { after: { // @x-ts-expect-error https://github.com/microsoft/TypeScript/issues/55709 + // TypeScript limitation: mapped types with string unions don't narrow properly. + // A dev-mode runtime warning will be shown for this. unknown: 'b' } }, @@ -2043,6 +2045,8 @@ describe('setup()', () => { a: { after: { // @x-ts-expect-error https://github.com/microsoft/TypeScript/issues/55709 + // TypeScript limitation: mapped types with string unions don't narrow properly. + // A dev-mode runtime warning will be shown for this. unknown: 'b' } },