Skip to content

feat: add experimental workspace parameters page for dynamic params #17841

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
May 21, 2025
Merged

feat: add experimental workspace parameters page for dynamic params #17841

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
dd83a00
feat: add experimental workspace parameters page for dynamic params
jaaydenh May 14, 2025
f19178c
fix: update type
jaaydenh May 15, 2025
0c7f4c1
chore: get parity with create workspace page
jaaydenh May 19, 2025
17a2c85
fix: pass form value to dynamic parameter
jaaydenh May 20, 2025
93359a0
chore: move useSyncFormParameters to shared hook
jaaydenh May 20, 2025
82e0b07
chore: use Map to improve performance
jaaydenh May 21, 2025
e827b2c
chore: move hook inside modules
jaaydenh May 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
chore: use Map to improve performance
  • Loading branch information
@jaaydenh
jaaydenh committed May 21, 2025
commit 82e0b075b3e8dda878f693619e9f3df3be9abe1f
18 changes: 7 additions & 11 deletions site/src/hooks/useSyncFormParameters.ts
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this isn't a very general purpose hook. could we put this in modules/workspaces or something instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hook will eventually be used a many different places, TemplateEmbedPage, WorkspaceSettingsPage, CreateWorkspacePage, UpdateBuildParametersDialog, BuildParametersPopover, WorkspaceParametersForm. Its not general purpose but it also doesn't feel like it should be buried in a subfolder. Maybe we don't currently have a good place for things like this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to modules/hooks, modules are the place to gather all coder business logic that is not in pages so seems like the best place to put coder specific non-general purpose hooks

Original file line number Diff line number Diff line change
Expand Up @@ -29,22 +29,18 @@ export function useSyncFormParameters({
if (!parameters) return;
const currentFormValues = formValuesRef.current;

const newParameterValues = parameters.map((param) => {
return {
const newParameterValues = parameters.map((param) => ({
name: param.name,
value: param.value.valid ? param.value.value : "",
};
});
}));

const currentFormValuesMap = new Map(
currentFormValues.map((value) => [value.name, value.value]),
);

const isChanged =
currentFormValues.length !== newParameterValues.length ||
newParameterValues.some(
(p) =>
!currentFormValues.find(
(formValue) =>
formValue.name === p.name && formValue.value === p.value,
),
);
newParameterValues.some((p) => !currentFormValuesMap.has(p.name) || currentFormValuesMap.get(p.name) !== p.value);

if (isChanged) {
setFieldValue("rich_parameter_values", newParameterValues);
Expand Down