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

(feat) - replaceNode parameter #1557

Merged

Conversation

LukasBombach
Copy link
Contributor

@LukasBombach LukasBombach commented Apr 21, 2019

This implementation brings back the third optional parameter replaceNode from 8.x

render(component, containerNode, [replaceNode])

https://preactjs.com/guide/api-reference#preact-render-

With this parameter you can provide child node in your parent DOM that will be used as render root. It does not replace other children inside the parent and it will hydrate the provided node and not just replace it, the DOM nodes remain the same.

I made a more comprehensive infographic to describe why I see this as useful

https://gist.github.com/LukasBombach/884319d5430a3fb85f3b4385d7a31c89

You can try out a working usage in this repo: https://github.com/spring-media/pool-attendant (See index.partial.hydration.js)

ADDS: 16B to core

@coveralls
Copy link

coveralls commented Apr 21, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling 238e0e4 on LukasBombach:feat/replaceNodeParameter into 7be38b4 on developit:master.

src/render.js Show resolved Hide resolved
@LukasBombach LukasBombach changed the title Feat/replace node parameter (feat) replaceNode parameter Apr 21, 2019
@LukasBombach LukasBombach changed the title (feat) replaceNode parameter (feat) - replaceNode parameter Apr 21, 2019
@JoviDeCroock JoviDeCroock self-requested a review April 21, 2019 10:19
Copy link
Member

@JoviDeCroock JoviDeCroock left a comment

Choose a reason for hiding this comment

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

This looks like a good addition to me 💯

expect(scratch.innerHTML).to.equal('<div id="a"></div><div id="b"></div><div id="c"></div>');
});

it('should render multiple render roots in one parentDom', () => {
Copy link
Member

Choose a reason for hiding this comment

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

This test is so good 👍 💯

Copy link
Contributor Author

Choose a reason for hiding this comment

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

❤️

Copy link
Member

@marvinhagemeister marvinhagemeister left a comment

Choose a reason for hiding this comment

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

Love it! Was super awesome to see it grow on slack. The infographic is wonderful and really shows the benefits of this change. Thank you so much for making the PR, highly appreciated 👍 🥇

@marvinhagemeister marvinhagemeister merged commit 3b7d356 into preactjs:master Apr 22, 2019
@marvinhagemeister
Copy link
Member

@LukasBombach Congrats on getting your first PR merged 🎉

@LukasBombach LukasBombach deleted the feat/replaceNodeParameter branch April 22, 2019 13:11
@LukasBombach
Copy link
Contributor Author

LukasBombach commented Apr 22, 2019

Thank you so much! I am really happy and proud! And this feature will be so useful to us!

@prateekbh
Copy link
Member

@developit this should fix our problem with SSR in preact-cli, right?

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.

5 participants