-
Notifications
You must be signed in to change notification settings - Fork 46.9k
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
Generating random/unique attributes server-side that don't break client-side mounting #4000
Comments
Have you considered either doing some concatenation of the props passed in? Additionally, on the client side you could store all active ids in a map or WeakSet and throw an error or warning if it already exists. Another alternative is setting up something like brianmcd/contextify to keep both sides synchronized per request. |
Good question. I don't know the answer. |
Yeah, in this case the
It's actually fairly easy to guarantee that the client-side ids are kept unique (the code I posted will do that OK), unless I'm misunderstanding your suggestion.
It would probably take me a bit longer to grok how exactly that would tie in as a solution, but at any rate I would like to think there's a lighter-weight (read: dependency free) solution for this. The main approaches I've considered so far are:
So yeah, that leads me to this issue. If there's not a great solution to this, I'd be curious to hear any ideas on how React might support this use case in a future release. This is obviously a very specific example I'm giving, but I'd imagine it could be solved through a more general approach that enables components to dynamically generate their own server-side property/state values, and then perform a successful mounting on the client. |
Mostly brainstorming here, so I'd be curious to hear what the rest of the team thinks (and nothing I say here constitutes an official recommendation). Personally, I would go with option number 2. What you're really looking for is a UUID, but you don't want to use random because then the markup wouldn't match :/. Ultimately, this is the server-side-clock problem (ie. suppose you had a component that rendered the current time displaying microsecond accuracy; the two machines are going to have clock skew, making it impossible to generate matching markup). Incidentally, since When framing the problem as the server-side-clock problem, I think it's pretty clear that you want to have some way of controlling the current time (perhaps you want the clock to be red&green on christmas, and have eggs&bunnies on easter) because you need a way to test this functionality more than once per year. To write unit tests to verify the functionality, you would need to control the time. The difference is that, for your component, your need for a unique identifier is an implementation detail not visible to the user, so the dependency on sideways data kinda sucks. Eeh, not sure what to do about that. In the near future, you will be able to get around the cumbersome passing-down-props problem by using In the more distant future, you will be able to use sideways data loading, perhaps in combination with context, to subscribe to external data like the clock. @jmar777 does that help a little? |
This might be a stretch, but the same approach while leveraging |
@jimfb Technically I think it really just comes down to the need for a facility for centralizing per-root data (and optionally automatically transmitting server to client), i.e. conceptually just a per-root object that everyone can access. Everything else related to this issue (like time, random numbers, etc) can then be implemented in-terms of that functionality, it's really rather unopinionated and flexible. It wouldn't even have to be strictly exposed by React, you can do this right now, but for a reusable component ecosystem to thrive I think it has to be. One could imagine React providing only per-root UIDs, UIDs, seeds, etc through it's API, but it's possibly too opinionated (but possibly sufficient) and I can imagine many benefits to having a preferred facility for centralizing and transmitting necessary data. |
@syranide I think that's the point/goal of I suppose context needs to be initialized by the user, so you can't have "first user is responsible for initializing" style code (you need to have a root |
@jimfb I wouldn't say so, To put it differently, this issue seems "trivially" solvable to me (in theory):
You can do all of this today, but it's cumbersome especially if you isolate it per component and especially if you don't have any kind of framework to help you, each component has to reinvent all the wheels (and also send more data). I'm not saying this exact solution is it or where to draw the line of what React should provide and not. But I think this illustrates the issue, it's already doable, but it's not really realistic. |
@syranide Right, but isn't propagation of data the only thing we are able to handle? I mean, without dictating the server side architecture (or at least transport protocol), we aren't able to help with the "source" of the data. The data needs to come from the server (and management of that is the domain of some other framework, like Relay or other). It seems like we can only reliably solve 5 and 7. We could maybe solve 9 (and potentially help with 4+7) with sideways data loading. But I think 1,2,3,4,6 are all far outside the domain of React (maybe in the domain of related projects like Relay). I'm not sure if React can/should help with anything beyond the propagation of data. Or am I wrong and/or miss-interpreting your point? I'm curious which of the above steps you think we should be responsible for handling? |
That is the question I think :)
It seems like we're somewhat on the same page. To me it seems there are 4 options:
Looking at that it seems obvious that option 4 is overreaching. It seems to me that this could be considered a basic necessity like To be clear, you can replace "React" with "Library X" if you want (this is not rocket science), it would be democratic but it seems it would be hurtful to the ecosystem for reusable components. No one wants to deal with "Library X & Y & Z" just because the various components chose a different library even though they all have largely the same basic purpose. Option 3. Just so that I'm abundantly clear (hopefully), what I'm suggesting is just the idea of the somewhat pseudo-code below which is the solution to this issue (and in the context of this discussion, function getPerRootUID(context) {
if (context.dataStore.rootUID === undefined) {
context.dataStore.rootUID = Math.random();
}
return context.dataStore.rootUID;
}
var rootUIDCounters = {};
function incrementPerRootCounter(context) {
var rootUID = getPerRootUID(context);
if (rootUIDCounters[rootUID] === undefined) {
rootUIDCounters[rootUID] = 0;
}
return rootUIDCounters[rootUID]++;
}
class InitialDataStoreWrapper {
constructor(props, state, context) {
this.context.dataStore = this.props.data;
}
}
class AppWithGUID {
constructor(props, state, context) {
this.state = {
guid: incrementPerRootCounter(context);
};
}
}
// Server
var dataStoreObject = {};
var markup = React.renderToString(
<InitialDataStoreWrapper data={dataStoreObject}>
<AppWithGUID />
</InitialDataStoreWrapper>
);
/* transfer markup and dataStoreObject to client */
// Client
var dataStoreObject = /*dataStoreObject from server*/;
var dataStoreWrapper = React.render(
<InitialDataStoreWrapper data={dataStoreObject}>
<AppWithGUID />
</InitialDataStoreWrapper>
); I hope it gets my point across at least (it's total crap code and not correct at all). So again, I'm not at all saying this is the solution (it isn't), I'm just trying to illustrate the utility in there being something like this. Also, I'm not advocating that this should extend to per-component data (usually associated with model data, Relay, Flux, etc). I'm only suggesting this as a basic utility for allowing components to execute deterministically for the purpose of GUID generation, random numbers, etc. Just shout if this made no sense at all... |
@syranide Makes sense to me. There have actually been some talks internally about requiring all IO go through React. And that things like getTime (and therefore, transitively, getRandomNumber) would count as IO. I think the idea was proposed mostly for academic purposes (a thought exercise, not a proposal for implementation), but this might add fuel to that discussion. Suppose we provide a UUID generator, and we provide a time api, and we provide... [we can create a huge list of possible APIs to provide, but how do we know we've covered everyone else's use cases?]. My preference is to provide basic building blocks, from which people can build whatever they want/need. So the question is: what is the common theme / underlying API that would make it easier to implement these things in user land? Where does context/sideways-data fall short of what is needed? More brainstorming: What if we (somehow, magically) allowed a context variable to have a default value. A component could require a dependency on a Would that be sufficient, if we could somehow make it work? Or would that not solve the problem? |
Those are interesting discussions, but I'm curious exactly what you mean by "requiring all IO go through React", what kinds of IO is it referring to? (Or is it in the sense that no data should exist outside of components, like Flux stores does? So all data can be serialized and is guaranteed to represent the entire state?) ... My replies below are purely gut feeling, intuition and brainstorming :)
The way I see it, if you give me (per-root) Time, a Seed, a UUID and a way to store per-root local data. With that a component should be able to accomplish any useful deterministic process without requiring any additional data be sent from client to server. UUID is only necessary for producing markup with GUIDs, which seems like a necessity only because of HTML deficiencies and should not otherwise be useful I think... but it might have other uses. I think the key point of these features is there is no benefit in having more that one definition/source of each, it's probably only detrimental even. But these are values that have to be transferred from server to client, if React does not provide these then components has to make these demands on other libraries or on the consumer of the component, neither of which seems at all beneficial (because it's redundant, leads to duplication of data and code, inconsistencies, etc). But Time, Seed and UUID have fairly limited usefulness to reusable components and when those are needed they could be provided on an as-needed basis by the consumer if we want to be hands-off (but UUID is intrinsically useful to some reusable ReactDOM components). So I think the primary issue is that per-root local data simply doesn't exist currently, because even if you have Seed you're unable to use it for anything stateful because you have nowhere appropriate to store the data without putting explicit requirements on the consumer for making specific per-root data storage available through context (which is a slippery slope from a maintenance perspective).
So yeah it seems to me that (technically) it is indeed a solution to the above lack of "root local data", which is the primary problem. 👍 What to do about Time, Seed and UUID is a secondary problem. We may decide not to provide some/all in core, but I think it would be a mistake not to at least make them available as separate "preferred" modules (possibly even just as interfaces). So that reusable components can specify them as dependencies. But again, less of an immediate concern. PS. I think |
Like I said, mostly academic exercises (we were only half-serious about the idea). That said, there were a couple of variants of the idea. What if we didn't allow a component's data to enter/exit the jsvm without passing through React. Or maybe you manage the layer between the component and the flux store. The point is: if we manage the IO, React gains a lot of power to do super interesting things. For instance, if you replay all io since the beginning of time, you can restore (rewind) the React tree to any point in time. No idea if that would be useful, but it leads to interesting conclusions. UUIDs have many other use cases. For instance, suppose (as a result of a click event) you want to create a new data object or a new transaction to be sent to the server. You want an identifier for lookup or to prevent accidental replays when doing retries. But if we only allow the supplied With regards to making it deterministic... this is potentially the hard part, as component render order is not guaranteed by the React API. We may (in the not-so-distant-future) start rendering in parallel via workers. Yeah, clock/seed/uuid all need to be functions/datastores/something-more-complex-than-a-constant. Especially since they will need to be able to handle things like out-of-order renders, making the naive implementations of things like seed/uuid non-deterministic. My intuition is that the solution somehow involves a default value for a context variable, but we honestly haven't given it enough thought yet, so I don't know how such a thing would work. |
I think it seems like an interesting exercise in determining if there are better ways developers should structure their apps. Perhaps not so much as a future direction for the React API in particular. As I mentioned in the other gist, IMHO the strength of React is how close feature-wise it is to manually wiring UIs without the associated burdens. It even does so without being opinionated. (I know academic, just saying :))
I thought about that but I feel like it's an overreach, these UUIDs would be for the purpose of separating React roots (for the benefit of reusable components). So it seems to me that UUIDs for any other purpose than that is outside the scope of React and provides no benefit (communication is not something reusable components is concerned with, they only communicate through callbacks).
As far as I've understood it, only
Only clock would be a dynamic value, seed and UUID would be static values per root... although the seed provided to the consumer probably should be mutated per use (so also dynamic) to avoid separate implementations of the same random number generator algorithm producing the same sequences separately.
Yeah, default value === inject context variable, so it seems like a solution, but exactly how it should be implemented to make sense is an interesting problem for sure (you don't want every component specifying/providing their own default...). |
soooo I tried to read through this issue but most of the conversation is going over my head :p so if this has been answered, forgive me. Is there currently a reasonable workaround to this problem that react component libraries can leverage? I see a bunch of options for app builders, but not so much for libraries that want to generate IDs for their components but have no control over the entire render tree many thanks |
I've experimented with a solution for making this available through a common interface. var ReactRootStore = require('ReactRootStore');
var testProperty = ReactRootStore.createProperty(0);
var HelloWorld = React.createClass({
contextTypes: ReactRootStore.contextTypes,
render: function() {
var initialValue = testProperty.get(this);
testProperty.set(this, 3);
return <div>Hello {initialValue} {testProperty.get(this)}</div>;
}
});
// The root element must be wrapped so the context variable is exposed
React.render(ReactRootStore.wrapElement(function() {
return <HelloWorld />;
}), document.getElementById('container')); https://github.com/syranide/react-root-store - It works. It's possible the code in the repo has some minor error/bug, I only committed it to get it out there for now. https://jsfiddle.net/tfttkyfm/ JSFiddle with slightly older code, but it works and shows the concept. This should provide a store that behaves deterministically when going from server to client as it is per-root and not global. |
Closing this in favor of more recent discussion in #5867. |
This PR is to fix the errors `Warning: Prop "id" did not match. Server: "some-id". Client: "another-id"`, reported in https://yext.slack.com/archives/C032CKFARGS/p1664916209295449 React expects that the **rendered content is identical between the server and the client**. However, our usage of `uuid` means different id is generated from the HTML content generated through SSR vs the first render in client side during hydration process. This is a known issues in React SSR (issues in react repo[[1](https://github.com/facebook/react/issues/4000)][[2](https://github.com/facebook/react/issues/5867)][[3](https://github.com/facebook/react/issues/1137)]) and React released a new hook `useId` in React 18 to address it. I found three potential solutions for React <18 to **generate unique but predictable id**, each with an external library we could use: 1) Use context to keep a counter in state, so counter is not shared across renders. This approach is implemented in [react-aria](https://www.npmjs.com/package/react-aria). When using SSR with React Aria, applications must be wrapped in an `SSRProvider` component to reset the count in server side to be zero for each fresh render. Multiple copies of React Aria is not supported so it would be a peer-dep ``` //in component lib (dropdown) import { useId } from 'react-aria'; ... const screenReaderUUID: string = useId(); //in pageJS template using SSR import { SSRProvider } from 'react-aria'; ... <SSRProvider> <TestComponent /> </SSRProvider> ``` 2) use a global counter to increment on initial renders and expose a reset function for SSR side. This approach is implemented in [react-id-generator](https://www.npmjs.com/package/react-id-generator). There's a function `resetId` required to invoke from SSR side (such as in PageJS `serverRenderRoute` or somewhere in user's template page) to avoid mismatch during refresh, since browser generator will always restart from "1" but server's count will keep incrementing. 3) Don't use IDs at all on the server; patch after first render. This approach is implemented in [reach/auto-id.](https://www.npmjs.com/package/@reach/auto-id) ID returned as undefined/empty on the first render so server and client have the same markup during hydration process. After render, patch the components with an incremented ID (second render) in useLayoutEffect. (more info [here](https://github.com/reach/reach-ui/blob/dev/packages/auto-id/src/reach-auto-id.ts)) No changes needed outside of using `import { useId } from '@reach/auto-id';` in our component lib. I decided to go with the last approach using `react/auto-id` because: - requires no additional work / changes from user side (only in component lib) - contain fallback implementation to React 18 `useId` if it's available - avoid potential concurrent rendering problem (such as suspense boundaries) - the main issue with this approach is that it causes a double render on any components with `useId`. However, since we (1) only update the ID attribute on DOM on second render, and (2) our components that uses `useId` already requires multiple renders to get setup, the performance is not greatly affected. I used Profiler in our test-site to measure performance, the number of renders and time spent for `SearchBar`, `StaticFilters`, and `FilterSearch` did not change as the dispatch from `useId` is grouped with other changes required in our components on second render. This is only needed for React <18 J=SLAP-2403 TEST=manual&auto `npm pack` a build of the component lib with the new changes and tested SearchBar, FilterSearch, and StaticFilers: - in the starter PageJS project (https://github.com/yext/pages-starter-react-locations) with React 17 - in another [yext-sites-starter](https://github.com/tmeyer2115/yext-sites-starter) repo with React 18 override See that the warning about Prop `id` mismatched no longer appear in console added new jest test for Dropdown and StaticFilter, see that test failed the previous usage of uuid, and passed with reach/auto-id.
Consider, for example, a relatively generic
<Field />
component:In order for the
<label />
tag to be semantically related to the<input />
tag, we obviously need to match thefor
attribute to the<input />
tag'sid
attribute. Outside of that requirement, however, we have no need of theid
attribute, so a solution like the one above is convenient as it handles that all internally, with no need for a consuming component to pass in an actualid
value.The problem I'm running into, however, is that this causes the
id
attributes generated client-side to mismatch what was sent down by the server (the client-sidefieldCounter
restarts at0
on every load, whereas the server-side reference obviously just keeps growing).This mismatch then results in an error being thrown:
Invariant Violation: You're trying to render a component to the document using server rendering [...]
So, my question is this: am I overlooking an obvious solution here? I would like for the
<Field />
component to be able to simply internalize theid
generation as an implementation detail, but I can't seem to come up with a good mechanism for then matching up that id generation client-side.Thanks in advance!
The text was updated successfully, but these errors were encountered: