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

data-reactid alternative. #5869

Closed
DylanPiercey opened this issue Jan 17, 2016 · 12 comments
Closed

data-reactid alternative. #5869

DylanPiercey opened this issue Jan 17, 2016 · 12 comments

Comments

@DylanPiercey
Copy link

I know 0.15.0 no longer uses keys in data-reactids which is part of my issue however I'd like to bring forward a different way to link the dom for the initial render which is to keep text nodes and regular nodes separate.

I have implemented something like this in my own simple virtual dom here https://github.com/DylanPiercey/tusk/blob/master/lib/virtual/node.js#L80 and here https://github.com/DylanPiercey/tusk/blob/master/lib/virtual/text.js#L37

I am not sure if react works this way at all but essentially it recursively mounts nodes from the top down and splits up text nodes using splitText to ensure that the server produced dom matches the virtual structure.

Just curious if something like this is feasible in react since it has allowed me to implement bootstrapping server side code while producing clean html output without checksums or ids.

@gaearon
Copy link
Collaborator

gaearon commented Apr 1, 2016

Would you be interested in creating a proof of concept of this in React?

@DylanPiercey
Copy link
Author

@gaearon unfortunately I'm not very familiar with the internals of React. I was more hoping that someone with a good understanding of React's internals would draw inspiration from the lib I linked.

@gaearon
Copy link
Collaborator

gaearon commented Apr 2, 2016

Unfortunately server rendering is not currently a priority for Facebook so we can’t dedicate enough time to optimizations related to it.

If you give it a try, we might be able to help you along the way. See #5753 as an example of an ambitious PR made by an external contributor that significantly changed how React renders text nodes. I would imagine that if you were to look into this, you might at least touch some similar files.

We plan to introduce some internal documentation (#6335) but we’re not there yet. We are however enthusiastic about seeing people contribute in some areas where we don’t have enough resources. Please let us know if you’d like to give it a try, and feel free to create a work-in-progress PR where you can ask questions as you stumble into problems or need a clarification.

@DylanPiercey
Copy link
Author

@gaearon Thanks for the links #5753 does look like something I can work with. Hopefully I will have time at some point to give this a shot.

@catamphetamine
Copy link
Contributor

catamphetamine commented Oct 2, 2016

@DylanPiercey And what for is the data-reactid attribute anyway?
There's no information about it on the internet.

@DylanPiercey
Copy link
Author

@halt-hammerzeit the data-reactid attribute was partially removed in 15.x. Basically it was what was used to link up the dom with react elements. Now it is only used to link up react when you are using server side rendering.

@catamphetamine
Copy link
Contributor

catamphetamine commented Oct 3, 2016

@DylanPiercey Well, while I can understand why it could be used on the client (and this article seems to kinda explain it) I don't see a reason why does it exist on the server.
The client gets the markup from the server, without any data-reactids, then it renders the same stuff on the client, without data-reactids, then it compares the two strings and throws a warning if they don't match including the very very short and uninformative piece of diff like this

Warning: React attempted to reuse markup in a container but the checksum was invalid. This generally means that you are using server rendering and the markup generated on the server was not what the client was expecting. React injected new markup to compensate which works but you have lost many of the benefits of server rendering. Instead, figure out why the markup being generated is different on the client or server:
 (client) rgin:0;display:flex;-webkit-align-items:
 (server) rgin:0;display:flex;align-items:center;j

And that's it. No data-reactids needed. No data-react-checksums needed. Just extract the data-reactroot innerHTML and compare it as a string.

@DylanPiercey
Copy link
Author

DylanPiercey commented Oct 3, 2016

@halt-hammerzeit there are plenty of virtual doms that don't need these id attributes to bootstrap existing DOM so it is definitely not required. Unfortunately I don't think react puts too much effort into SSR though (according to @gaearon). They seem to have an "if it works, leave it" attitude.

@catamphetamine
Copy link
Contributor

catamphetamine commented Oct 3, 2016

@DylanPiercey Yeah, I guess that's because facebook is an isolated ecosystem rather than a part of the internet.
So they don't need "Web is documents" concept at all.

I myself was thinking about React server side rendering in terms of caching, and came to a conclusion that it would require a rewrite of the current react-dom/server package to drop data-reactids and data-react-checksums in favour of assembling a React page from different totally independent pieces of raw HTML markup.
Like, you cache a <TopComments/> block and you also cache <TopArticles/> block and then you assemble your <ReactPage/> from a mix of React elements and pieces of raw HTML markup.
Currently React doesn't provide such possibilty: it only can render the whole thing one at a time, guarding it with data-reactids and data-react-checksums.
I guess someone will eventually develop such a mixed caching solution, and probably that could be a big company that hits performance issues while rendering React on the server-side.
For now I'm gonna stick with this strategy: only render SEO-specific blocks on the server side and then render all the variable blocks (user bar, comments, etc) on the client side in componentDidMount.
This way React won't complain with the "attempted to reuse" warning and all pages can be safely cached on the server side as raw HTML strings (and this cache will only be invalidated when the data changes which is supposed to be infrequent).

@aweary
Copy link
Contributor

aweary commented Oct 3, 2016

Currently React doesn't provide such possibilty: it only can render the whole thing one at a time, guarding it with data-reactids and data-react-checksums.
I guess someone will eventually develop such a mixed caching solution, and probably that could be a big company that hits performance issues while rendering React on the server-side.

You might want to check out react-dom-stream, which does some caching with SSR

@catamphetamine
Copy link
Contributor

catamphetamine commented Oct 3, 2016

@aweary Thanks, looks like it's the way to go (in the future)
(even though this particular project seems to be abandoned and doesn't support React 15)

@gaearon
Copy link
Collaborator

gaearon commented Aug 1, 2017

We don't use IDs anymore so this can probably close. #10339
The change will be part of React 16.

@gaearon gaearon closed this as completed Aug 1, 2017
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

No branches or pull requests

4 participants