feat: handle update build for dynamic params #18226
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
Adds support for dynamic parameters during workspace build updates by wiring a feature flag through the UI, hooks, and API layers.
- Introduce
isDynamicParametersEnabledflag in batch actions, update dialogs, and mutation calls - Provide an opt-out hook (
useDynamicParametersOptOut) and branch between classic/experimental parameter flows in multiple components - Extend API methods to fetch and apply “dynamic parameters” when the feature is enabled
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| site/src/modules/workspaces/WorkspaceUpdateDialogs.tsx | Wire dynamic-params feature flag into update workspace mutation and dialogs |
| site/src/hooks/useDynamicParametersOptOut.ts | New hook to read opt-out state from localStorage |
| site/src/api/api.ts | Add getDynamicParameters and branch in updateWorkspace/changeWorkspaceVersion for dynamic vs. rich parameters |
| site/src/api/queries/workspaces.ts | Pass isDynamicParametersEnabled through changeVersion and updateWorkspace query definitions |
| site/src/pages/** | Pass feature flag and templateVersionId through workspace pages and dialogs, refactor various routers |
| site/src/components/Dialog/Dialog.tsx | Expose additional exports and update spacing/typography in dialog primitives |
Comments suppressed due to low confidence (2)
site/src/modules/workspaces/WorkspaceUpdateDialogs.tsx:60
- [nitpick] The boolean flag is named
dynamicParametersEnabledhere butisDynamicParametersEnabledelsewhere; consider standardizing on theis*prefix for consistency.
const dynamicParametersEnabled = experiments.includes("dynamic-parameters");
site/src/hooks/useDynamicParametersOptOut.ts:1
- New feature logic in this hook isn’t covered by tests; consider adding unit tests for the opt-out default behavior and localStorage reads.
import { useQuery } from "react-query";
The updateWorkspace function now requires additional parameters: - buildParameters (array, defaults to []) - isDynamicParametersEnabled (boolean, defaults to false) Updated all test calls to include these parameters to fix test failures.
When the dynamic-parameters experiment is not enabled, the parameter dialogs were showing a loader indefinitely instead of the classic parameter dialog. This was causing e2e tests to fail because they couldn't find the parameter fields. Fixed by: - Only showing loader when experiment is enabled AND optOut data is loading - Using classic dialog when experiment is disabled OR user opted out - This ensures e2e tests work correctly without the experiment flag
Emyrk
approved these changes
Jun 5, 2025
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
resolves coder/preview#110