-
Notifications
You must be signed in to change notification settings - Fork 47k
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
Comments
Would you be interested in creating a proof of concept of this in React? |
@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. |
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 And what for is the |
@halt-hammerzeit the |
@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.
And that's it. No |
@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. |
@DylanPiercey Yeah, I guess that's because facebook is an isolated ecosystem rather than a part of the internet. 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 |
You might want to check out |
@aweary Thanks, looks like it's the way to go (in the future) |
We don't use IDs anymore so this can probably close. #10339 |
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.
The text was updated successfully, but these errors were encountered: