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

fix(editor): Fix design system typecheck errors (no-changelog) #9447

Merged
merged 8 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,16 @@ const props = withDefaults(defineProps<ColorPickerProps>(), {
showAlpha: false,
colorFormat: 'hex',
popperClass: '',
predefine: undefined,
modelValue: undefined,
showInput: true,
name: uid('color-picker'),
});

const color = ref(props.modelValue);

const colorPickerProps = computed(() => {
const { showInput, ...rest } = props;
const { showInput, modelValue, size, ...rest } = props;
return rest;
});

Expand All @@ -40,6 +42,8 @@ const emit = defineEmits<{
(event: 'active-change', value: string): void;
}>();

const resolvedSize = computed(() => props.size as '' | 'small' | 'large' | 'default' | undefined);

const onChange = (value: string) => {
emit('change', value);
};
Expand All @@ -61,7 +65,7 @@ const onColorSelect = (value: string) => {
<span :class="['n8n-color-picker', $style.component]">
<ElColorPicker
v-bind="colorPickerProps"
size="default"
:size="resolvedSize"
@change="onChange"
@active-change="onActiveChange"
@update:model-value="onColorSelect"
Expand All @@ -70,7 +74,7 @@ const onColorSelect = (value: string) => {
v-if="showInput"
:class="$style.input"
:disabled="props.disabled"
:size="props.size"
:size="size"
:model-value="color"
:name="name"
type="text"
Expand Down
36 changes: 25 additions & 11 deletions packages/design-system/src/components/N8nFormInput/FormInput.vue
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
<template>
<N8nCheckbox
v-if="type === 'checkbox'"
v-bind="$props"
ref="inputRef"
:label="label"
:disabled="disabled"
:label-size="labelSize as CheckboxLabelSizePropType"
:model-value="modelValue as CheckboxModelValuePropType"
@update:model-value="onUpdateModelValue"
@focus="onFocus"
/>
Expand All @@ -17,7 +20,7 @@
{{ tooltipText }}
</template>
<ElSwitch
:model-value="modelValue"
:model-value="modelValue as SwitchModelValuePropType"
:active-color="activeColor"
:inactive-color="inactiveColor"
@update:model-value="onUpdateModelValue"
Expand Down Expand Up @@ -59,9 +62,9 @@
v-else
ref="inputRef"
:name="name"
:type="type"
:type="type as InputTypePropType"
:placeholder="placeholder"
:model-value="modelValue"
:model-value="modelValue as InputModelValuePropType"
:maxlength="maxlength"
:autocomplete="autocomplete"
:disabled="disabled"
Expand Down Expand Up @@ -99,7 +102,18 @@
import { ElSwitch } from 'element-plus';

import { getValidationError, VALIDATORS } from './validators';
import type { Rule, RuleGroup, IValidator, Validatable, FormState } from '../../types';
import {

Check failure on line 105 in packages/design-system/src/components/N8nFormInput/FormInput.vue

View workflow job for this annotation

GitHub Actions / Lint changes

All imports in the declaration are only used as types. Use `import type`
Copy link
Contributor

Choose a reason for hiding this comment

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

lint issue here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Updated.

Rule,
RuleGroup,
IValidator,
Validatable,
InputModelValuePropType,
InputTypePropType,
SwitchModelValuePropType,
CheckboxModelValuePropType,
CheckboxLabelSizePropType,
InputAutocompletePropType,
} from '../../types';

import { t } from '../../locale';

Expand All @@ -120,10 +134,10 @@
validators?: { [key: string]: IValidator | RuleGroup };
maxlength?: number;
options?: Array<{ value: string | number; label: string; disabled?: boolean }>;
autocomplete?: string;
autocomplete?: InputAutocompletePropType;
name?: string;
focusInitially?: boolean;
labelSize?: 'small' | 'medium';
labelSize?: 'small' | 'medium' | 'large';
disabled?: boolean;
activeLabel?: string;
activeColor?: string;
Expand Down Expand Up @@ -206,7 +220,7 @@
$emit('blur');
}

function onUpdateModelValue(value: FormState) {
function onUpdateModelValue(value: Validatable) {
state.isTyping = true;
$emit('update:modelValue', value);
}
Expand All @@ -225,9 +239,9 @@
const error = getInputValidationError();

if (error) {
if (error.messageKey) {
return t(error.messageKey, error.options);
} else {
if ('messageKey' in error) {
return t(error.messageKey, error.options as object);
} else if ('message' in error) {
return error.message;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export const requiredValidator: IValidator<{}> = {
};

export const minLengthValidator: IValidator<{ minimum: number }> = {
validate: (value: Validatable, config: { minimum: number }) => {
validate: (value: Validatable, config) => {
if (typeof value === 'string' && value.length < config.minimum) {
return {
messageKey: 'formInput.validator.minCharactersRequired',
Expand Down Expand Up @@ -76,7 +76,7 @@ export const emailValidator: IValidator<{}> = {
};

export const containsUpperCaseValidator: IValidator<{ minimum: number }> = {
validate: (value: Validatable, config: { minimum: number }) => {
validate: (value: Validatable, config) => {
if (typeof value !== 'string') {
return false;
}
Expand All @@ -94,7 +94,7 @@ export const containsUpperCaseValidator: IValidator<{ minimum: number }> = {
};

export const matchRegex: IValidator<{ regex: RegExp; message: string }> = {
validate: (value: Validatable, config: { regex: RegExp; message: string }) => {
validate: (value: Validatable, config) => {
if (!config.regex.test(`${value as string}`)) {
return {
message: config.message,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export default defineComponent({
default: true,
},
tagSize: {
type: String,
type: String as PropType<'small' | 'medium'>,
default: 'small',
validator: (value: string): boolean => ['small', 'medium'].includes(value),
},
Expand Down
25 changes: 21 additions & 4 deletions packages/design-system/src/components/N8nInput/Input.vue
Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
<template>
<ElInput
ref="innerInput"
:size="computedSize"
:model-value="modelValue"
:size="resolvedSize"
:class="['n8n-input', ...classes]"
:autocomplete="autocomplete"
:name="name"
v-bind="{ ...$props, ...$attrs }"
:placeholder="placeholder"
:disabled="disabled"
:readonly="readonly"
:clearable="clearable"
:rows="rows"
:title="title"
v-bind="$attrs"
>
<template v-if="$slots.prepend" #prepend>
<slot name="prepend" />
Expand All @@ -27,6 +34,7 @@
import { ElInput } from 'element-plus';
import { uid } from '../../utils';
import type { InputSize, InputType } from '@/types/input';
import { InputAutocompletePropType } from '@/types';

Check failure on line 37 in packages/design-system/src/components/N8nInput/Input.vue

View workflow job for this annotation

GitHub Actions / Lint changes

All imports in the declaration are only used as types. Use `import type`
Copy link
Contributor

Choose a reason for hiding this comment

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

lint issue here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Updated.


interface InputProps {
modelValue?: string | number;
Expand All @@ -40,12 +48,13 @@
maxlength?: number;
title?: string;
name?: string;
autocomplete?: 'off' | 'autocomplete';
autocomplete?: InputAutocompletePropType;
}

defineOptions({ name: 'N8nInput' });
const props = withDefaults(defineProps<InputProps>(), {
modelValue: '',
type: 'text',
size: 'large',
placeholder: '',
disabled: false,
Expand All @@ -58,7 +67,15 @@
autocomplete: 'off',
});

const computedSize = computed(() => (props.size === 'xlarge' ? undefined : props.size));
const resolvedSize = computed(
() =>
(props.size === 'xlarge' ? undefined : props.size) as
Copy link
Contributor

Choose a reason for hiding this comment

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

why is type coercion needed here if we already set InputSize in InputProps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ElementPlus components have a different type declared. Technically they accept any value for the prop and will generate a class based on it, but vue-tsc will complain because it's not the right PropType.

Copy link
Contributor

Choose a reason for hiding this comment

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

If it makes sense here or too complicated feel free to keep.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure I can fix it in a more elegant way other than casting unfortunately.

| ''
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe this size type could be an imported type? since it's the same as in ColorPicker and InputNumber

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great point. Updated.

| 'small'
| 'large'
| 'default'
| undefined,
);

const classes = computed(() => {
const applied: string[] = [];
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<script setup lang="ts">
import type { InputSize } from '@/types';
import { ElInputNumber } from 'element-plus';
import { computed } from 'vue';

type InputNumberProps = {
size?: InputSize;
Expand All @@ -10,9 +11,24 @@ type InputNumberProps = {
precision?: number;
};

defineProps<InputNumberProps>();
const props = withDefaults(defineProps<InputNumberProps>(), {
size: undefined,
step: 1,
precision: 0,
min: -Infinity,
max: Infinity,
});

const resolvedSize = computed(() => props.size as '' | 'small' | 'large' | 'default' | undefined);
</script>

<template>
<ElInputNumber v-bind="{ ...$props, ...$attrs }" />
<ElInputNumber
:size="resolvedSize"
:min="min"
:max="max"
:step="step"
:precision="precision"
v-bind="$attrs"
/>
</template>
4 changes: 3 additions & 1 deletion packages/design-system/src/components/N8nLink/Link.vue
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import type { TextSize } from '@/types/text';
const THEME = ['primary', 'danger', 'text', 'secondary'] as const;

interface LinkProps {
to?: RouteLocationRaw;
to?: RouteLocationRaw | string;
size?: TextSize;
newWindow?: boolean;
bold?: boolean;
Expand All @@ -27,6 +27,8 @@ interface LinkProps {

defineOptions({ name: 'N8nLink' });
withDefaults(defineProps<LinkProps>(), {
to: undefined,
size: undefined,
bold: false,
underline: false,
theme: 'primary',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,12 @@
<template #content>{{ nodeTypeName }}</template>
<div v-if="type !== 'unknown'" :class="$style.icon">
<img v-if="type === 'file'" :src="src" :class="$style.nodeIconImage" />
<FontAwesomeIcon v-else :icon="name" :class="$style.iconFa" :style="fontStyleData" />
<FontAwesomeIcon
v-else
:icon="name as string"
:class="$style.iconFa"
:style="fontStyleData"
/>
</div>
<div v-else :class="$style.nodeIconPlaceholder">
{{ nodeTypeName ? nodeTypeName.charAt(0) : '?' }}
Expand All @@ -22,7 +27,7 @@
<template v-else>
<div v-if="type !== 'unknown'" :class="$style.icon">
<img v-if="type === 'file'" :src="src" :class="$style.nodeIconImage" />
<FontAwesomeIcon v-else :icon="name" :style="fontStyleData" />
<FontAwesomeIcon :icon="name as string" :style="fontStyleData" />
Copy link
Contributor

Choose a reason for hiding this comment

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

could we avoid type coercion here? and above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

<div v-if="badge" :class="$style.badge" :style="badgeStyleData">
<n8n-node-icon :type="badge.type" :src="badge.src" :size="badgeSize"></n8n-node-icon>
</div>
Expand Down
6 changes: 3 additions & 3 deletions packages/design-system/src/components/N8nRoute/Route.vue
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
<template>
<router-link v-if="useRouterLink" :to="to" v-bind="$attrs">
<router-link v-if="useRouterLink && to" :to="to" v-bind="$attrs">
<slot></slot>
</router-link>
<a v-else :href="to" :target="openNewWindow ? '_blank' : '_self'" v-bind="$attrs">
<a v-else :href="to as string" :target="openNewWindow ? '_blank' : '_self'" v-bind="$attrs">
Copy link
Contributor

Choose a reason for hiding this comment

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

could we avoid type coercion? maybe check if it's a string in v-else-if

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

<slot></slot>
</a>
</template>
Expand All @@ -12,7 +12,7 @@ import { computed } from 'vue';
import { type RouteLocationRaw } from 'vue-router';

interface RouteProps {
to?: RouteLocationRaw;
to?: RouteLocationRaw | string;
newWindow?: boolean;
}

Expand Down
15 changes: 9 additions & 6 deletions packages/design-system/src/components/N8nSelect/Select.vue
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,16 @@ export default defineComponent({
},
computed: {
listeners() {
return Object.entries(this.$attrs).reduce<Record<string, () => {}>>((acc, [key, value]) => {
if (/^on[A-Z]/.test(key)) {
acc[key] = value;
}
return Object.entries(this.$attrs).reduce<Record<string, (...args: unknown[]) => {}>>(
(acc, [key, value]) => {
if (/^on[A-Z]/.test(key)) {
acc[key] = value as (...args: unknown[]) => {};
Copy link
Contributor

Choose a reason for hiding this comment

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

same question here. could we refactor this to avoid type coercion?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

}

return acc;
}, {});
return acc;
},
{},
);
},
computedSize(): InnerSelectRef['$props']['size'] {
if (this.size === 'medium') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ export default {
};

const methods = {
onReinvite: action('reinvite'),
onDelete: action('delete'),
action: ({ action: actionName }: { action: string; userId: string }) => action(actionName),
};

const Template: StoryFn = (args, { argTypes }) => ({
Expand All @@ -23,8 +22,7 @@ const Template: StoryFn = (args, { argTypes }) => ({
components: {
N8nUsersList,
},
template:
'<n8n-users-list v-bind="args" :actions="actions" @reinvite="onReinvite" @delete="onDelete" />',
template: '<n8n-users-list v-bind="args" :actions="actions" @action="action" />',
methods,
});

Expand Down
11 changes: 8 additions & 3 deletions packages/design-system/src/components/N8nUsersList/UsersList.vue
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ interface UsersListProps {

const props = withDefaults(defineProps<UsersListProps>(), {
readonly: false,
currentUserId: '',
users: () => [],
actions: () => [],
isSamlLoginEnabled: false,
Expand Down Expand Up @@ -101,11 +102,15 @@ const defaultGuard = () => true;
const getActions = (user: IUser): UserAction[] => {
if (user.isOwner) return [];

return props.actions.filter((action) => (action.guard || defaultGuard)(user));
return props.actions.filter((action) => (action.guard ?? defaultGuard)(user));
};

const $emit = defineEmits(['*']);
const onUserAction = (user: IUser, action: string) => $emit(action, user.id);
const $emit = defineEmits(['action']);
const onUserAction = (user: IUser, action: string) =>
$emit('action', {
action,
userId: user.id,
});
</script>

<style lang="scss" module>
Expand Down
Loading
Loading