Thunberg087 / vue-fragment

a very candide fragment component for Vue.js
http://jsfiddle.net/cdkn5wL3/
670 stars 51 forks source link

Let fragment could be used in v-for #31

Closed kerlw closed 2 years ago

y-nk commented 5 years ago

That sounds useful. I wonder if there's any side effects... I'll try to review it soon but i'm honestly overloaded with work :/ if someone else wants to try/test it, please also try.

kerlw commented 5 years ago

My commit solved two problems when using fragment in v-for:

  1. When update the count of fragment in v-for changed, Vue would remove some fragment from the parent, BUT your solution would restore the original removeChild for parent after remove the first fragment, so Vue could not remove other fragment correctly.
  2. When change the sequence of fragment in v-for (because each fragment might has a 'key', and this 'key' maybe from data, and data maybe sort with other field), Vue would use insertBefore to change the fragment's position, I just use previousSibling and nextSibling as double linked queue to operate fragments
kerlw commented 5 years ago

I found a bug, argument 'ref' of insertBefore might be null, so I added a judgement

rakista112 commented 5 years ago

I tested this. I put a fragment with 2 list items along with a regular component in a ul element. I rendered the list with v-for and it worked as I expected. It even fixed my error "Failed to execute 'removeChild' on 'Node': The node to be removed is not a child of this mode." in #32 .

I for one suggest that this be pulled.

Just be sure to update browserslist. npm i --save browserslist. The current commit says that caniuse-lite is outdated forever.

kerlw commented 5 years ago

I found this solution has a critical defect in complex hierarchical structure I'm working on another two solutions:

  1. base on this solution, trying to perfect it
  2. trying to use vue functional component
kerlw commented 5 years ago

nested fragment may cause excetions when Vue update vnodes

I found in latest Vue, fragment could be implemented by using functional component:

Vue.component('fragment', {
    functional: true,
    render: function (h, context) {
        let dom = h('div', context.data, context.children)
        return dom.children || []
    }
});
y-nk commented 5 years ago

@kerlw could you provide a working jsfiddle ? I tried : https://jsfiddle.net/bqfoky5u/1/ but it won't work.

kerlw commented 5 years ago

I'm so sorry, I'v made a stupid mistake. This solution only works in our project, because each component would be converted by our converter, and wrapped by a element, so it dose work. I cannot open jsfiddle, but I tried in a vue project, the multi-root error occurs.

So, here is the old one fragment I've modified, trying to solve nested fragment problems, not solved perfectly, but does work in most situation: https://gist.github.com/kerlw/e8579089f6dc8fa48ad9089772b5255e

kerlw commented 5 years ago

I'v commit my code to the pull request, I hope it would help you.

shaniqwa commented 4 years ago

please merge :)

jackprosser commented 4 years ago

Tried using your version @kerlw and I get a freeze undefined.

y-nk commented 4 years ago

@shaniqwa did you use the fork without problem ?

mikhailian commented 4 years ago

There is a pure HTML solution for rendering a tree like a table using css properties table, table-row and table-cell mixed with root elements. See this fiddle for the example.

martinfruehmorgen commented 4 years ago

Hello,

@kerlw thank you, that works if you add const freeze = (object, property, value) => { Object.defineProperty(object, property, { configurable: true, get () { return value }, set (v) { console.warn(tried to set frozen property ${property} with ${v}) } }) }

const unfreeze = (object, property, value = null) => { Object.defineProperty(object, property, { configurable: true, writable: true, value: value }) }

at the root of index.js

Thank again, Martin

j0nathan33 commented 4 years ago

Any update for this pull request

Tofandel commented 4 years ago

This PR needs to be cleaned up, there shouldn't be all this stuff in index.js, indentation is also apocalyptic

yushko2015 commented 3 years ago

I need this fixed also. When will be merged?

y-nk commented 3 years ago

@yushko2015 as @Tofandel mentioned the PR doesn't look like fitting. You can fork the fork and propose your PR on the side?