Skip to content

Conversation

dobromir-hristov
Copy link
Contributor

This is just an example how one can extend VTU with either normal prototype attachment, or via a dedicated API.

Please give me your opinions on which one you think is better?

import { mount, VueWrapper, config } from '../../src'
import Hello from '../components/Hello.vue'
// add a method to the VueWrapper
declare module '../../src/vue-wrapper' {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

IDK if thats how you should do it, just used the first thing I saw.

Copy link
Member

Choose a reason for hiding this comment

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

I think this is something users would normally do in an index.d.ts file on the top level

}
}

VueWrapper.prototype.name = function () {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The cool thing here is, all the instanced will get the function as its native.

return this.componentVM.$.type.name
}

config.plugins.VueWrapper.lowerCaseName = function lowerCaseName(
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 is a little easier to grasp for people with little prototype knowledge, but has a little higher memory cost I guess?

Copy link
Member

Choose a reason for hiding this comment

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

I think people who want to write an extension prob know enough to learn about a prototype

Copy link
Member

@lmiller1990 lmiller1990 left a comment

Choose a reason for hiding this comment

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

looks pretty good

plugins: {
VueWrapper: {} as any,
DOMWrapper: {}
}
Copy link
Member

Choose a reason for hiding this comment

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

I will look into typing this! I want to get better as TS, but I think this is fine for now, nice


// plugins hook
Object.entries(config.plugins.VueWrapper).forEach(
([name, handler]: [string, () => any]) => {
Copy link
Member

Choose a reason for hiding this comment

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

lol, nice! you can also use unknown for the return type, reflecting an unknown value, both fine

import { mount, VueWrapper, config } from '../../src'
import Hello from '../components/Hello.vue'
// add a method to the VueWrapper
declare module '../../src/vue-wrapper' {
Copy link
Member

Choose a reason for hiding this comment

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

I think this is something users would normally do in an index.d.ts file on the top level

return this.componentVM.$.type.name
}

config.plugins.VueWrapper.lowerCaseName = function lowerCaseName(
Copy link
Member

Choose a reason for hiding this comment

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

I think people who want to write an extension prob know enough to learn about a prototype

@dobromir-hristov
Copy link
Contributor Author

@afontcu @JessicaSachs What is your opinion?

Should we promote prototype extension or create a dedicated api, like the one I have proposed here? I think i am aiming at the prototype one.

@afontcu
Copy link
Member

afontcu commented Apr 14, 2020

Should we promote prototype extension or create a dedicated api, like the one I have proposed here? I think i am aiming at the prototype one.

hey! 👋 Not 100% sure, actually 😇 I'm a bit concerned with the actual implementation of both examples:

return this.componentVM.$.type.name vs return this.name().toLocaleLowerCase()

the former looks way more complicated to grasp than the latter. Are those two equivalent? Is there a way to simplify the first one?

update: i'm just dumb XD

Thanks for this!

@dobromir-hristov
Copy link
Contributor Author

Haha no no, that was just a way to test whether they work. One uses the method added prior.

How you access the internals is the same in both cases.

@afontcu
Copy link
Member

afontcu commented Apr 14, 2020

Haha no no, that was just a way to test whether they work. One uses the method added prior.
How you access the internals is the same in both cases.

Yeah okay makes sense hahaha.

I do think that relying on prototypes "feels" hacky, even though I'm OK with this given that it's an advanced feature and that it should be documented 👍

@lmiller1990
Copy link
Member

lmiller1990 commented Apr 15, 2020

So if we are going with the VueWrapper.prototype for now, do we just need to export VueWrapper and that's it? I'm happy to go with that is everyone else is - no point to provide a special API, JavaScript already provides one we all know about (prototype).

@dobromir-hristov
Copy link
Contributor Author

We just need to export DOMWrapper and VueWrapper and that should be it. We should also test this after it is transpiled, so that means when it is published as a package.

@lmiller1990
Copy link
Member

Ok, want to update this to just export those two?

I made a ticket for some e2e tests: #68

@JessicaSachs JessicaSachs self-requested a review April 17, 2020 02:35
Copy link
Contributor

@JessicaSachs JessicaSachs left a comment

Choose a reason for hiding this comment

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

I love this. I don't find the prototype approach hacky at all. I think this even gives us the flexibility to write plugins with Vue?!

@lmiller1990
Copy link
Member

@dobromir-hristov should we export DOMWrapper too?

@afontcu
Copy link
Member

afontcu commented Apr 17, 2020

I guess this would become the "right" approach to extend VTU as Vue Testing Library does? I mean, the way to add "custom" matchers and other stuff. I'll try to give this a go during the weekend and come back to you folks with the result 👍

@JessicaSachs
Copy link
Contributor

JessicaSachs commented Apr 17, 2020

So I just finished toying around. I found the API a bit awkward at first. I'll see if there's a pattern that feels more natural. I mostly found that I just wanted the this context in a closure while I was extending VueWrapper.

Before:

config.plugins.VueWrapper.name = function name() {
  return this.componentVM.$.type.name
}

config.plugins.VueWrapper.lowerCaseName = function lowerCaseName() {  
  return this.name().toLocaleLowerCase()
}

Possible solution to enclose the wrapper instance?

config.plugins.VueWrapper.extend((wrapper) => ({
  lowerCaseName: () => wrapper.name().toLocaleLowerCase(),
  name: () => wrapper.componentVM.$.type.name
})

The other thing I'd like to solve is the ability to use reactive properties so that not everything has to be a method. I'll continue toying with that.

@dobromir-hristov
Copy link
Contributor Author

So no prototype extension?

Private field would not be accessible if passed as a param.

@JessicaSachs
Copy link
Contributor

You would still apply it to the prototype when you install the plugin in the constructor. I'm mostly just concerned with getting a reference to wrapper.

@lmiller1990
Copy link
Member

Are we leaning towards @JessicaSachs implementation for a plug-in or this one? Let's pick one and focus on that. I am excited to write some plug-ins (mainly findByTestId).

@lmiller1990 lmiller1990 merged commit c8f119e into master Apr 24, 2020
@dobromir-hristov dobromir-hristov deleted the feature/extending-without-plugin branch April 27, 2020 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants