-
Notifications
You must be signed in to change notification settings - Fork 3.2k
fix(autofill-env-vars): simplify/remove logic to not cause useEffect overwrites #726
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
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.
Greptile Summary
This PR simplifies the environment variable autofill logic by removing a complex, problematic implementation in favor of a more straightforward approach.
The key changes are:
- Removes the
autoFillEnvVarsfeature and related UI components, marking it as deprecated in the database schema - Eliminates complex state management using
useEffecthooks that were causing race conditions - Implements a simpler provider-based API key persistence logic:
- API keys persist when switching between models of the same provider
- API keys clear only when switching to a different provider
The changes span across multiple areas including database schema, stores, API endpoints, and React components, removing the complex state tracking system while maintaining a good user experience.
Confidence score: 4/5
- This PR is safe to merge as it simplifies existing logic without introducing new complexity
- High confidence due to comprehensive cleanup across the codebase, but missing new tests reduces the score slightly
- Key files needing attention:
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/components/sub-block/hooks/use-sub-block-value.ts- Core logic changesapps/sim/db/schema.ts- Schema changes and deprecationapps/sim/stores/workflows/subblock/types.ts- State management simplification
11 files reviewed, 2 comments
Edit PR Review Bot Settings | Greptile
...]/w/[workflowId]/components/workflow-block/components/sub-block/hooks/use-sub-block-value.ts
Show resolved
Hide resolved
|
✅ No security or compliance issues detected. Reviewed everything up to b62317f. Security Overview
Detected Code Changes
Reply to this PR with |
…overwrites (simstudioai#726) * fix(autofill-env-vars): simplify logic to not cause useEffect overwrites * remove useless comments
Description
Simplify Autofill Env Var logic.
Previously: Had a store mapping that overrode API Key using a useEffect based on provider. Detected whether it was cleared by the user and marked clears. Didn't work because of race conditions between useEffects and mapping was unreliable.
Simplified behavior: Don't clear API Key on switching between same providers -- so convenience is still there to pick model within same provider after API key entered. But clear when switching to different provider. Requires no complex state management.
Fixes # (issue)
How Has This Been Tested?
Screen.Recording.2025-07-19.at.4.46.36.PM.mov
Checklist:
bun run test)Security Considerations: