Suggestion: un-deprecate attachToDocument

[alexmiddeleer]

I feel the most common use case of attachTo is still attachToDocument, since that is necessary for testing any Vue code that utilizes document.querySelector and similar methods, even if run in total isolation. By un-deprecating attachToDocument, we prevent developers who are new to the library from having to hunt for the following code.

    const elem = document.createElement('div')
    if (document.body) {
      document.body.appendChild(elem)
    }
   wrapper = mount(Component, {
    attachTo: elem
   })

[afontcu]

Hi! 👋 Thanks for bringing this up. While it is true that attaching to document is probably the most common scenario, attachTo provides more flexibility to cover other cases, so we'd want to stick with it.

OTOH, it would be great to add this snippet in docs. Would you like to do so? 😄

[alexmiddeleer]

Sure, I can do that. Thanks for considering.

…

On Fri, Jun 12, 2020 at 6:42 PM Adrià Fontcuberta ***@***.***> wrote: Hi! 👋 Thanks for bringing this up. While it is true that attaching to document is probably the most common scenario, attachTo provides more flexibility to cover other cases, so we'd want to stick with it. OTOH, it would be great to add this snippet in docs. Would you like to do so? 😄 — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1578 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABJGQP64SNLMWBW434HEHTTRWKVM5ANCNFSM4N4LX2MQ> .

[just-boris]

+1 to the undeprecation suggestion.

What is the reason for all users to write 3 lines of boilerplate, if this can be an extra built-in option? What are the risks of having both attachTo and attachToDocument?

[Jeff-Duke]

+1 to the un-deprecation suggestion.
why unnecessarily increase boilerplate? why not have both methods?

[lmiller1990]

Supporting both of these seems pretty reasonable. I think the original PR was to make it more robust, but we did not consider the additional boilerplate that would be introduced.

I wonder if we can have the best of both worlds?

mount(Foo, {
  attachTo: someEl
})

// what about adding support for?

mount(Foo, {
  attachTo: document
})

Thoughts? We get the best of both words, and it reads pretty nicely too. We could just check if value of the attachTo property is a document, and if it is, create a new element, append it to document.body and mount there.

@Jeff-Duke, @just-boris, @afontcu?

[just-boris]

As far as I understand how attachTo currently works, it is replacing the DOM node with the content from Vue. I tried using it with mount(Foo, { attachTo: document.body}) and it broke, because document.body becomes null for subsequent tests.

Having a separate explicit method is preferred because it is more clear than the difference between attachTo: document.body and attachTo: htmlElement behavior.

[afontcu]

I feel it might be hard for newcomers to understand the differences between attachTo, attachToDocument, and even not using them at all.

That being said, if people feel the need to have this baked in, I'm okay with it 👍 as suggested right above, I'd keep it separate from attachTo (unless we find a way to make attachTo and attachToDocument behave similarly!)

[lmiller1990]

Ok so we just re added the previous method back in? Should be easy. Would anyone like to take responsibility for this?

[JessicaSachs]

I personally prefer having attachTo just work with document.body and to solve the null error. I was just working with this a few days ago for Cypress' Component Testing solution (which wraps VTU)

[lmiller1990]

There are use cases for attaching to a specific element apparently (adding Vue into existing applications with pre-existing HTML...)

[vvanpo]

In vue@3 and @vue/test-utils@2 using attachTo: document.body works just fine, as Vue 3's app.mount() method works differently from Vue 2's app.$mount(), where the former appends an element to the mount point whereas the latter replaces it. So if we wanted some consistency between the behaviours in version 1 vs. 2, supporting attachTo: document.body and doing something like if (attachTo instanceof HTMLBodyElement) { // append child <div /> as mountpoint might help future migrations a lot more than un-deprecating attachToDocument (which would be redundant to have in version 2).

[lmiller1990]

We should make the migration process as seamless as possible.

I thikn we should port the above suggestion, and make attachTo: document.body work in this codebase too. It's basically the same amount of typing as attachToDocument: true and much more flexible. A PR implementing this would be welcome.

[lmiller1990]

This was merged in #1699 and will go out in a few days.