-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: Save generators without file dialog #77
Conversation
|
||
export function Generator() { | ||
const name = useGeneratorStore((store) => store.name) | ||
const filteredRequests = useGeneratorStore(selectFilteredRequests) | ||
const hasRecording = useGeneratorStore(selectHasRecording) | ||
const [isLoading, setIsLoading] = useState(false) | ||
const { path } = useParams() | ||
invariant(path, 'Path is required') |
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.
Common frontend implementation of the assert pattern. Looks concise and helps to narrow the type
|
||
const handleEdit = () => { | ||
selectRule(ruleId) | ||
navigate(`/generator/rule/${ruleId}`) | ||
navigate(`/generator/${encodeURIComponent(path)}/rule/${ruleId}`) |
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.
Apparently the previous PR broke this :\
We should probably start using functions to build paths like we do in GCk6
if (!har) { | ||
return | ||
} | ||
invariant(har, 'Failed to open file') |
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.
not convinced on the invariant use on this, feels like we should just better handle the error case, but let's discuss it in the tech retro
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.
Yes, definitely! This should never actually fire, but in general, invariant is just a "wrapper" for
if (!value) {
throw new Error(message)
}
So it will be caught by whatever error handlers we'll be adding over the course of the upcoming weeks
No description provided.