-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
"Durations defined in setup() are not strongly typed " #5399
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tool you used to create this PR is serving you very poorly. I'd like to help you understand why I think that and how to improve. Consider my review to be a review of the tool's output, not a review of the code changes you've made.
To start, the PR description is overly wordy and does nothing to help reviewers review your code.
The goal of a PR description is to provide:
- what changes you've made
- why you made those changes
- (optional): how to use the changes
How is optional because it's only relevant if the PR effects changes to the public API of the project (like how to import a newly-exported function, for example).
What, why, and (sometimes) how are important because they provide reviewers with the context they need to understand the code they're reviewing.
This is the only part of the PR description that adds any context:
Added a warning that triggers when an unknown delay key is referenced
The rest is irrelevant information.
Additionally, this PR contains multiple files that shouldn't have been committed because the tool created them for you, not for us or for xstate.
I've added some additional comments to the review that hopefully explain what's wrong here and how they could be done better.
|
sorry for the troble i should have checked |
|
@Sambhram1 you're totally fine, just trying to help you open a good pr. The change you're trying to make seems reasonable to me, it's just all the extra stuff that's not helping you out! |
Changes Made
Added Development-Mode Warning (raise.ts)
fixes:[#5375]
Added a warning that triggers when an unknown delay key is referenced
Only runs in development mode (no production overhead)
Provides helpful message listing available delays
Updated Type Test Documentation (setup.types.test.ts)
Added clarifying comments explaining the TypeScript limitation (TS#55709)
Notes that runtime warnings provide the safety net
Added Comprehensive Tests (delays.test.ts - NEW)
5 test cases covering all scenarios
Tests warning behavior for unknown delays
Tests no warnings for valid configurations
Documents immediate transition behavior
Key Benefits
✅ Better Developer Experience - Clear warnings catch mistakes during development
✅ Zero Production Cost - Warnings only in development mode
✅ Fully Backward Compatible - No breaking changes
✅ Well Documented - Tests and comments explain the TypeScript limitation
Documentation Created
PR_DESCRIPTION.md - Comprehensive PR description for GitHub
PR_CHANGES.md - Technical details of the changes
SUMMARY.md - Quick reference for the PR