-
Notifications
You must be signed in to change notification settings - Fork 258
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: record native events against each wrapper they bubble through #394
Changes from 1 commit
c58ffa4
770133d
1ba4e59
cc59f55
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,7 +12,7 @@ import { createWrapperError } from './errorWrapper' | |
import { TriggerOptions } from './createDomEvent' | ||
import { find, matches } from './utils/find' | ||
import { mergeDeep, textContent } from './utils' | ||
import { emitted } from './emit' | ||
import { emitted, recordEvent } from './emit' | ||
|
||
export class VueWrapper<T extends ComponentPublicInstance> { | ||
private componentVM: T | ||
|
@@ -30,6 +30,9 @@ export class VueWrapper<T extends ComponentPublicInstance> { | |
this.rootVM = vm?.$root | ||
this.componentVM = vm as T | ||
this.__setProps = setProps | ||
|
||
this.attachNativeEventListener() | ||
|
||
config.plugins.VueWrapper.extend(this) | ||
} | ||
|
||
|
@@ -42,6 +45,21 @@ export class VueWrapper<T extends ComponentPublicInstance> { | |
return this.vm.$el.parentElement | ||
} | ||
|
||
private attachNativeEventListener(): void { | ||
const vm = this.vm | ||
if (!vm) return | ||
Comment on lines
+55
to
+56
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Calling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I cannot imagine why this would happen 🤔 I will look at it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh it makes sense, functional components can be found with most likely someone is going to ask about capturing emitted events from functional components. let's deal with it if and when it comes up 🤷 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test I added has a functional component for the grandparent, which works because it captures events that bubbled to the element of the outer wrapper, I suppose. It might be different for functional components without their own native DOM element but I suppose in that case DOM events wouldn't expect to be captured? |
||
|
||
const element = this.element | ||
for (let key in element) { | ||
if (/^on/.test(key)) { | ||
const eventName = key.slice(2) | ||
element.addEventListener(eventName, (...args) => { | ||
recordEvent(vm.$, eventName, args) | ||
}) | ||
} | ||
aethr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
|
||
get element(): Element { | ||
// if the component has multiple root elements, we use the parent's element | ||
return this.hasMultipleRoots ? this.parentElement : this.vm.$el | ||
|
@@ -79,7 +97,7 @@ export class VueWrapper<T extends ComponentPublicInstance> { | |
} | ||
|
||
emitted<T = unknown>(): Record<string, T[]> | ||
emitted<T = unknown>(eventName?: string): undefined | T[] | ||
emitted<T = unknown>(eventName: string): undefined | T[] | ||
emitted<T = unknown>( | ||
eventName?: string | ||
): undefined | T[] | Record<string, T[]> { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps this method should be exported from
emit.ts
withthis
as an argument, the style looks very different from anything else so I tried to add a private method instead. There is definitely some bleeding of concerns.