Skip to content

Commit

Permalink
fix(vue): ensure v-model is updated before event callback (#400)
Browse files Browse the repository at this point in the history
BREAKING CHANGE: The "externalEvent" key has been removed from `ComponentModelConfig`. The behavior that this enabled is now handled internally, so the key is no longer needed.
  • Loading branch information
liamdebeasi authored Oct 30, 2023
1 parent 1c8f4d4 commit bf05d4b
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 49 deletions.
14 changes: 1 addition & 13 deletions packages/vue-output-target/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,17 +72,6 @@ export interface ComponentModelConfig {
* of the `v-model` reference is based off.
*/
targetAttr: string;

/**
* (optional) The event to emit from the Vue component
* wrapper. When listening directly to the `event` emitted
* from the Web Component, the `v-model` reference has not
* yet had a chance to update. By setting `externalEvent`,
* your Web Component can emit `event`, the Vue output target
* can update the `v-model` reference, and then emit `externalEvent`,
* notifying the end user that `v-model` has changed. Defaults to `event`.
*/
externalEvent?: string;
}
```

Expand All @@ -94,8 +83,7 @@ vueOutputTarget({
componentModels: [
{
elements: ['my-input', 'my-textarea'],
event: 'v-on-change',
externalEvent: 'on-change',
event: 'on-change',
targetAttr: 'value'
}
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ export const MyComponent = /*@__PURE__*/ defineContainer<Components.MyComponent>
const generateComponentDefinition = createComponentDefinition('Components', [
{
elements: ['my-component'],
event: 'v-ionChange',
externalEvent: 'ionChange',
event: 'ionChange',
targetAttr: 'value',
},
]);
Expand Down Expand Up @@ -83,16 +82,15 @@ export const MyComponent = /*@__PURE__*/ defineContainer<Components.MyComponent,
'value',
'ionChange'
],
'value', 'v-ionChange', 'ionChange');
'value', 'ionChange');
`);
});

it('should add router and v-model bindings', () => {
const generateComponentDefinition = createComponentDefinition('Components', [
{
elements: ['my-component'],
event: 'v-ionChange',
externalEvent: 'ionChange',
event: 'ionChange',
targetAttr: 'value',
},
]);
Expand Down Expand Up @@ -143,7 +141,7 @@ export const MyComponent = /*@__PURE__*/ defineContainer<Components.MyComponent,
'value',
'ionChange'
],
'value', 'v-ionChange', 'ionChange');
'value', 'ionChange');
`);
});

Expand Down
4 changes: 0 additions & 4 deletions packages/vue-output-target/src/generate-vue-component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,6 @@ export const ${tagNameAsPascal} = /*@__PURE__*/ defineContainer<${componentType}

templateString += `,\n`;
templateString += `'${targetProp}', '${findModel.event}'`;

if (findModel.externalEvent) {
templateString += `, '${findModel.externalEvent}'`;
}
}

templateString += `);\n`;
Expand Down
1 change: 0 additions & 1 deletion packages/vue-output-target/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ export interface ComponentModelConfig {
elements: string | string[];
event: string;
targetAttr: string;
externalEvent?: string;
}

export interface PackageJSON {
Expand Down
49 changes: 24 additions & 25 deletions packages/vue-output-target/vue-component-lib/utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// @ts-nocheck
// It's easier and safer for Volar to disable typechecking and let the return type inference do its job.
import { VNode, defineComponent, getCurrentInstance, h, inject, ref, Ref } from 'vue';
import { defineComponent, getCurrentInstance, h, inject, ref, Ref, withDirectives } from 'vue';

export interface InputProps<T> {
modelValue?: T;
Expand Down Expand Up @@ -53,16 +53,13 @@ const getElementClasses = (
* to customElements.define. Only set if `includeImportCustomElements: true` in your config.
* @prop modelProp - The prop that v-model binds to (i.e. value)
* @prop modelUpdateEvent - The event that is fired from your Web Component when the value changes (i.e. ionChange)
* @prop externalModelUpdateEvent - The external event to fire from your Vue component when modelUpdateEvent fires. This is used for ensuring that v-model references have been
* correctly updated when a user's event callback fires.
*/
export const defineContainer = <Props, VModelType = string | number | boolean>(
name: string,
defineCustomElement: any,
componentProps: string[] = [],
modelProp?: string,
modelUpdateEvent?: string,
externalModelUpdateEvent?: string
modelUpdateEvent?: string
) => {
/**
* Create a Vue component wrapper around a Web Component.
Expand All @@ -78,29 +75,27 @@ export const defineContainer = <Props, VModelType = string | number | boolean>(
let modelPropValue = props[modelProp];
const containerRef = ref<HTMLElement>();
const classes = new Set(getComponentClasses(attrs.class));
const onVnodeBeforeMount = (vnode: VNode) => {
// Add a listener to tell Vue to update the v-model
if (vnode.el) {

/**
* This directive is responsible for updating any reactive
* reference associated with v-model on the component.
* This code must be run inside of the "created" callback.
* Since the following listener callbacks as well as any potential
* event callback defined in the developer's app are set on
* the same element, we need to make sure the following callbacks
* are set first so they fire first. If the developer's callback fires first
* then the reactive reference will not have been updated yet.
*/
const vModelDirective = {
created: (el: HTMLElement) => {
const eventsNames = Array.isArray(modelUpdateEvent) ? modelUpdateEvent : [modelUpdateEvent];
eventsNames.forEach((eventName: string) => {
vnode.el!.addEventListener(eventName.toLowerCase(), (e: Event) => {
el.addEventListener(eventName.toLowerCase(), (e: Event) => {
modelPropValue = (e?.target as any)[modelProp];
emit(UPDATE_VALUE_EVENT, modelPropValue);

/**
* We need to emit the change event here
* rather than on the web component to ensure
* that any v-model bindings have been updated.
* Otherwise, the developer will listen on the
* native web component, but the v-model will
* not have been updated yet.
*/
if (externalModelUpdateEvent) {
emit(externalModelUpdateEvent, e);
}
});
});
}
},
};

const currentInstance = getCurrentInstance();
Expand Down Expand Up @@ -146,7 +141,6 @@ export const defineContainer = <Props, VModelType = string | number | boolean>(
ref: containerRef,
class: getElementClasses(containerRef, classes),
onClick: handleClick,
onVnodeBeforeMount: modelUpdateEvent ? onVnodeBeforeMount : undefined,
};

/**
Expand Down Expand Up @@ -182,7 +176,12 @@ export const defineContainer = <Props, VModelType = string | number | boolean>(
}
}

return h(name, propsToAdd, slots.default && slots.default());
/**
* vModelDirective is only needed on components that support v-model.
* As a result, we conditionally call withDirectives with v-model components.
*/
const node = h(name, propsToAdd, slots.default && slots.default());
return modelProp === undefined ? node : withDirectives(node, [[vModelDirective]]);
};
});

Expand All @@ -199,7 +198,7 @@ export const defineContainer = <Props, VModelType = string | number | boolean>(

if (modelProp) {
Container.props[MODEL_VALUE] = DEFAULT_EMPTY_PROP;
Container.emits = [UPDATE_VALUE_EVENT, externalModelUpdateEvent];
Container.emits = [UPDATE_VALUE_EVENT];
}
}

Expand Down

0 comments on commit bf05d4b

Please sign in to comment.