Skip to content
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

Let fragment could be used in v-for #31

Closed
wants to merge 5 commits into from
Closed

Conversation

kerlw
Copy link
Contributor

@kerlw kerlw commented May 22, 2019

No description provided.

@y-nk
Copy link

y-nk commented May 23, 2019

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
Copy link
Contributor Author

kerlw commented May 23, 2019

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
Copy link
Contributor Author

kerlw commented May 24, 2019

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

@rakista112
Copy link

rakista112 commented Jul 15, 2019

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
Copy link
Contributor Author

kerlw commented Jul 18, 2019

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
Copy link
Contributor Author

kerlw commented Jul 22, 2019

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
Copy link

y-nk commented Jul 22, 2019

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

@kerlw
Copy link
Contributor Author

kerlw commented Jul 22, 2019

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 added 2 commits July 22, 2019 16:18
hook insertBefore/removeChild/appendChild
1. parent is fragment
2. child is a mounted fragment
1. use hooked Node's prototype functions to do the fragment logical
2. use document fragment to improve efficiency
@kerlw
Copy link
Contributor Author

kerlw commented Jul 22, 2019

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

@shaniqwa
Copy link

please merge :)

@jackprosser
Copy link

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

@y-nk
Copy link

y-nk commented Oct 3, 2019

@shaniqwa did you use the fork without problem ?

@mikhailian
Copy link

mikhailian commented Jan 23, 2020

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
Copy link

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
Copy link

Any update for this pull request

@Tofandel
Copy link

Tofandel commented Aug 4, 2020

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

@J-Rojas J-Rojas mentioned this pull request Aug 11, 2020
@yushko2015
Copy link

I need this fixed also.
When will be merged?

@y-nk
Copy link

y-nk commented Jan 28, 2021

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

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.