Skip to content
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: Vue Devtools #116

Merged
merged 27 commits into from
Feb 22, 2025
Merged

feat: Vue Devtools #116

merged 27 commits into from
Feb 22, 2025

Conversation

s8n11c
Copy link
Contributor

@s8n11c s8n11c commented Jan 29, 2025

No description provided.

Copy link

changeset-bot bot commented Jan 29, 2025

🦋 Changeset detected

Latest commit: 4ee0210

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@formwerk/devtools Patch
@formwerk/core Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@s8n11c s8n11c marked this pull request as draft January 29, 2025 02:44
@s8n11c s8n11c self-assigned this Jan 29, 2025
@s8n11c s8n11c requested a review from logaretm January 29, 2025 02:45
@logaretm logaretm changed the title Feat/dev tools feat: Vue DevTools Jan 29, 2025
@logaretm logaretm changed the title feat: Vue DevTools feat: Vue Devtools Jan 29, 2025
Copy link
Member

@logaretm logaretm left a comment

Choose a reason for hiding this comment

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

Some early thoughts

@s8n11c s8n11c requested a review from logaretm January 29, 2025 23:52
@logaretm logaretm linked an issue Feb 1, 2025 that may be closed by this pull request
@logaretm logaretm added 🩹 ver-patch PR should have a patch version changeset type 🌟 ver-minor PR should have a minor version changeset type and removed 🩹 ver-patch PR should have a patch version changeset type labels Feb 1, 2025
Copy link
Member

@logaretm logaretm left a comment

Choose a reason for hiding this comment

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

Several changes, nice work tho 👍

Comment on lines 303 to 308
watch(
() => ({
errors: errors.value,
isValid: isValid.value,
values: values,
isSubmitting: isSubmitting.value,
submitAttemptsCount: submitAttemptsCount.value,
}),
refreshInspector,
{
deep: true,
},
Copy link
Member

Choose a reason for hiding this comment

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

Move the watcher inside registerForm

@@ -36,6 +36,7 @@ export type FormField<TValue> = {
setTouched: (touched: boolean) => void;
setErrors: (messages: Arrayable<string>) => void;
displayError: () => string | undefined;
form?: FormContext | null;
Copy link
Member

Choose a reason for hiding this comment

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

rename this to resolvedForm for clarity.

@@ -191,7 +192,7 @@ export function useFormField<TValue = unknown>(opts?: Partial<FormFieldOptions<T
form.setFieldDisabled(path, disabled);
});

return field;
return { ...field, form };
Copy link
Member

Choose a reason for hiding this comment

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

No need to spread, just add the resolvedForm to the FormField type and then you could just set it as a prop here.

Comment on lines 6 to 43
export function registerTextFieldWithDevtools(field: Omit<TextField, 'type'>) {
const vm = initDevTools();

const id = field.getPath() ?? field.getName() ?? '';
const formId = field.form?.id;

// if the field is part of a form, we need to register to add the field to the form
if (formId && DEVTOOLS_FORMS[formId]) {
DEVTOOLS_FORMS[formId].children = DEVTOOLS_FORMS[formId].children ?? [];
DEVTOOLS_FORMS[formId].children.push({
...field,
type: FIELD_TYPES.TextField,
_vm: vm,
});
} else {
// if the field is a standalone field, we need to register it
DEVTOOLS_FIELDS[id] = { ...field, type: FIELD_TYPES.TextField, _vm: vm };
}

watch(
() => ({
errors: field.errorMessage.value,
isValid: !field.errorMessage.value,
value: field.fieldValue.value,
}),
refreshInspector,
{
deep: true,
},
);

onUnmounted(() => {
delete DEVTOOLS_FIELDS[id];
refreshInspector();
});

refreshInspector();
}
Copy link
Member

Choose a reason for hiding this comment

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

Avoid specific register functions for now, let's just start with registerField and registerForm, no need to have withDevtools as well because it is implied.

Change the function signature to be like this:

function registerField(field: FormField<unknown>, type: string) {
  // ....
}

Comment on lines 45 to 58
export type TextField = ReturnType<typeof useTextField> &
ReturnType<typeof useFormField<string | undefined>> & {
type: (typeof FIELD_TYPES)['TextField'];
};

export type CheckboxField = ReturnType<typeof useCheckbox> &
ReturnType<typeof useFormField<string | undefined>> & {
type: (typeof FIELD_TYPES)['Checkbox'];
};

export type RadioField = ReturnType<typeof useRadioGroup> &
ReturnType<typeof useFormField<string | undefined>> & {
type: (typeof FIELD_TYPES)['Radio'];
};
Copy link
Member

Choose a reason for hiding this comment

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

Won't need those if you follow my suggestion.

Comment on lines 10 to 28
export interface PathState<TValue = unknown> {
touched: boolean;
dirty: boolean;
valid: boolean;
errors: string[];
value: TValue;
}

// Form state extending base state
export interface FormState extends PathState {
id: string;
}

// Field state extending base state
export interface FieldState<TValue = unknown> extends PathState<TValue> {
path: string;
name: string;
formId?: string;
}
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to map types for those?

@logaretm logaretm assigned logaretm and unassigned s8n11c Feb 22, 2025
@logaretm logaretm marked this pull request as ready for review February 22, 2025 15:17
Copy link

pkg-pr-new bot commented Feb 22, 2025

Open in Stackblitz

npm i https://pkg.pr.new/formwerkjs/formwerk/@formwerk/core@116
npm i https://pkg.pr.new/formwerkjs/formwerk/@formwerk/devtools@116

commit: 23c3687

@logaretm logaretm merged commit d34984c into main Feb 22, 2025
3 checks passed
@logaretm logaretm deleted the feat/dev-tools branch February 22, 2025 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌟 ver-minor PR should have a minor version changeset type
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vue Devtools Plugin
2 participants