WIP: Add support for Fragments by marvinhagemeister · Pull Request #1080 · preactjs/preact

marvinhagemeister · 2018-04-28T19:04:16Z

This is really rough draft, but I thought to develop more in the open to get the ball rolling. The last test is still failing and the code feels really hacky, but hey it's a start 🎉

Fixes #946 .

calebeby · 2018-04-28T19:05:17Z


 });
+
+/** Fragmetn is a "passthrough" component which only has the key attribute


Spelled wrong 😉

Thanks for spotting this :)

coveralls · 2018-04-28T19:06:25Z

Coverage remained the same at 100.0% when pulling 4e2c553 on marvinhagemeister:fragments into 8be9c5f on developit:master.

marvinhagemeister · 2018-04-30T06:59:53Z

Tests are passing, even for array returns 🎉

Still need to figure out a cleaner to integrate all this.

marvinhagemeister · 2018-04-30T07:02:58Z

 				}
 			}
+			else {
+				updateChild(dom, child, f);


Likely a small performance loss, because updateChild is not inlined anymore. Need to measure this.

Compared with master my measurements don't show much differences. The empty diff test shows the most difference.

master branch

empty diff 89503/s (30 ticks) 84007/s (31 ticks)

repeat diff 11707/s (228 ticks) 11373/s (228 ticks)

large VTree 43276/s (62 ticks) 43877/s (58 ticks)

style/prop mutation 19974/s (133 ticks) 19745/s (132 ticks)

SSR Hydrate 1384/s (1962 ticks) 1364/s (1903 ticks)

Fixed the slowdown of the empty diff check. Problem was the Array.isArray check at https://github.com/developit/preact/pull/1080/files#diff-a0896fa5d65b1d80ca4d496badbae50fR106

how did you run those tests?

marvinhagemeister · 2018-04-30T07:03:22Z

 	}

+	if (Array.isArray(vnode)) {
+		return vnode.map(x => idiff(dom, x, context, mountAll, componentRoot));


TODO: Don't rely on Array.map

marvinhagemeister · 2018-04-30T07:04:28Z

+			test(<div><Comp /></div>);
+
+			expect(scratch.innerHTML).to.equal('<div><span>foo</span>bar</div>');
+		});


TODO: Add test for keyed Fragments

marvinhagemeister · 2018-05-05T07:44:55Z

Decided to remove straight array returns, because they are making everything needlessly complex. Just using Fragments for this leads to the same result, so introducing the additional complexity is not worth it.

staeke · 2018-06-16T12:41:26Z

Any news on this? Is it only awaiting approval?

tzvc · 2018-06-28T16:14:15Z

Any update on this guys ?

ergunsh · 2018-08-01T09:10:23Z

Any update?

arusanov · 2018-08-06T06:13:37Z

Any news?

Ayc0 · 2018-09-01T06:26:16Z

-	if (parent && ret.parentNode!==parent) parent.appendChild(ret);
+	if (parent) {
+		// append the element if its a new parent
+		if (ret instanceof Array) {


https://jsperf.com/instanceof-array-vs-array-isarray/33

can you try ret.constructor === Array?
its seems faster on many browsers
(you have two other instances of this in this file)

Ayc0 · 2018-09-01T15:44:36Z

@marvinhagemeister there is an issue with your PR: line 203, in the code, there is originalChildren = dom.childNodes, but has you mutate this NodeList when there is an array of VNode, the components aren't mounted in the right order
you can see that the behavior changes by replacing this line with originalChildren = Array.from(dom.childNodes) (which duplicates the nodelist)

Ayc0 · 2018-09-01T15:45:49Z

Without Array.from:

With Array.from:

Kanaye · 2018-09-01T15:53:44Z

Hi @Ayc0.
It's unlikely that this PR will be merged into preact.
As per @marvinhagemeister comment on #946:
There is already a mostly working version on a private repo for the next preact major version.

@marvinhagemeister If my assumption is right, can we close this pr with a comment/link to your comment in #946 ?

Ayc0 · 2018-09-01T16:04:04Z

@Kanaye perfect then 👌

marvinhagemeister · 2018-09-01T17:23:16Z

tldr; Fragments are part of the next major release 🎉 🎉

Sorry for being radio silence on this PR.

We do have merged Fragments into out private repo for the next major version of preact. Although work had already started there, I tried to shim Fragments into the current architecture against all odds. Unfortunately it didn't work out. This PR has issues when holey arrays or nested Fragments which is not easy to fix.

Our current architecture relies on a 1:1 relationship between a vnode and a dom element. Fragments obviously break that assumption. This and other features that we always wanted to implement were the reasons why we went out to rethink our core architecture.

@Kanaye yep this can be closed 👍

@Ayc0 you're right. Array.isArray is faster than instanceof Array now. It's awesome how much browsers have improved on that front 👍 Also good catch on the mounting issues. These are a bit tough to fix with the way preact currently works.

benjamin-schneider · 2018-10-01T08:01:38Z

Hi @marvinhagemeister, any chance to have a date for this next major release ? I searched on the whole website but did not manage to find this information. Thx

marvinhagemeister · 2018-10-01T08:58:48Z

@benjamin-schneider No we don't have set our eyes on a specific date. We're getting closer and closer each week but it's hard to predict an actual release date from that. Rest assured that all the hard work has been completed and we're currently working on getting all the ecosystem tools up to date (devtools, debug, render-to-string, etc).

You're right that this information can't be found on our website.

edimitchel · 2018-10-01T11:46:35Z

I waiting this feature too!
Thanks you for your amazing work !

Eli-Nathan · 2019-01-07T16:18:38Z

Any updates now on the date for the next major release?
This feature would save me right now 🙏

marvinhagemeister · 2019-01-07T17:35:28Z

@Eli-Nathan just tweeted an update on the current state of Preact X: https://twitter.com/marvinhagemeist/status/1082329232322252800

calebeby suggested changes Apr 28, 2018

View reviewed changes

marvinhagemeister mentioned this pull request Apr 30, 2018

Support (or document non-support) for Fragments #946

Closed

marvinhagemeister commented Apr 30, 2018

View reviewed changes

marvinhagemeister added 13 commits May 5, 2018 09:42

Add support for Fragments

9c113f6

Fix typo

88daf21

Add Fragment export

4a32c0e

Correctly patch children

1752cb2

Use for loop instead of forEach

43286f9

Add test for array returns

db0b68f

Update export tests

253c5f7

Replace Array.map

9590fd2

Add Fragment to TS typings

335ede9

Add failing test for Fragments with siblings

e404106

Make tests pass again

56f5641

Remove unused import

aa69404

Use instanceof Array instead of isArray

eac5fac

Fix minor code style issue

925b061

marvinhagemeister mentioned this pull request May 19, 2018

Decide what to do with upcoming react api changes #1047

Closed

9 tasks

Merge branch 'master' into fragments

e71096b

madeleineostoja mentioned this pull request Jun 6, 2018

[plugin-preact] Preact not compatible with Gatsby 2 (React 16) gatsbyjs/gatsby#5742

Closed

Merge branch 'master' into fragments

4e2c553

Kanaye mentioned this pull request Jul 3, 2018

Material-UI 1.2.1 not working with Preact preactjs/preact-compat#476

Closed

Ayc0 suggested changes Sep 1, 2018

View reviewed changes

marvinhagemeister closed this Sep 1, 2018

mjtognetti mentioned this pull request Sep 11, 2018

Stroke visual feedback inkandswitch/capstone#42

Merged


		});

		/** Fragmetn is a "passthrough" component which only has the key attribute

	master	branch
empty diff	89503/s (30 ticks)	84007/s (31 ticks)
repeat diff	11707/s (228 ticks)	11373/s (228 ticks)
large VTree	43276/s (62 ticks)	43877/s (58 ticks)
style/prop mutation	19974/s (133 ticks)	19745/s (132 ticks)
SSR Hydrate	1384/s (1962 ticks)	1364/s (1903 ticks)

Uh oh!

Conversation

marvinhagemeister commented Apr 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coveralls commented Apr 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marvinhagemeister commented Apr 30, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marvinhagemeister commented May 5, 2018

Uh oh!

staeke commented Jun 16, 2018

Uh oh!

tzvc commented Jun 28, 2018

Uh oh!

ergunsh commented Aug 1, 2018

Uh oh!

arusanov commented Aug 6, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ayc0 commented Sep 1, 2018

Uh oh!

Ayc0 commented Sep 1, 2018

Uh oh!

Kanaye commented Sep 1, 2018

Uh oh!

Ayc0 commented Sep 1, 2018

Uh oh!

marvinhagemeister commented Sep 1, 2018

tldr; Fragments are part of the next major release 🎉 🎉

Uh oh!

benjamin-schneider commented Oct 1, 2018

Uh oh!

marvinhagemeister commented Oct 1, 2018

Uh oh!

edimitchel commented Oct 1, 2018

Uh oh!

Eli-Nathan commented Jan 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marvinhagemeister commented Jan 7, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants

marvinhagemeister commented Apr 28, 2018 •

edited

Loading

coveralls commented Apr 28, 2018 •

edited

Loading

Eli-Nathan commented Jan 7, 2019 •

edited

Loading