facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
228.7k stars 46.81k forks source link

SSR: Cannot set property 'memoizedState' of null #16416

Open ambar opened 5 years ago

ambar commented 5 years ago

Do you want to request a feature or report a bug?

A bug?

What is the current behavior?

Cannot set property 'memoizedState' of null

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have dependencies other than React. Paste the link to your JSFiddle (https://jsfiddle.net/Luktwrdm/) or CodeSandbox (https://codesandbox.io/s/new) example below:

const processLink = html => {
  return renderToStaticMarkup(<Link />)
};

const RichText = ({ html }) => {
  const htmlProcessed = useMemo(() => processLink(html), [html]);
}

See https://codesandbox.io/s/cannot-set-property-memoizedstate-of-null-mrxfr

What is the expected behavior?

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

16.8~16.9

dhrubesh commented 5 years ago

@bvaughn Mind if I take this up?

bvaughn commented 5 years ago

Sure

ijxsid2 commented 4 years ago

Hi @bvaughn, this issue still exists. I can try this one unless someone else is not actively working on it.

gaearon commented 4 years ago

Commented: https://github.com/facebook/react/pull/17889#pullrequestreview-365914330. I don't think this fix is correct.

jeremyscatigna commented 4 years ago

Could I help here ? 🙂

gaearon commented 4 years ago

Sure.

jeremyscatigna commented 4 years ago

@gaearon Would it be possible to get some guidance on where to start ? I'm looking at the CodeSandbox provided to start digging in the problem and in the source possibly in ReactPartialRenderer.js and ReactPartialRendererHooks,js. Am I on the right path ?

bl00mber commented 4 years ago

Hey @gaearon it seems this bug may be caused by lack of isolation inside ReactPartialRendererHooks.js so when methods are executed they are changing the same variables for both renderers. If I'll try to isolate these variables within separate class is it will be an acceptable solution?

gaearon commented 4 years ago

@jeremyscatigna This sounds right. Are you still working on this?

hcharley commented 4 years ago

I believe this is happening to me using NextJS as well. I'm not sure exactly why the PR hasn't been merged. Not complaining, but just letting maintainers know this appears to be impacting a usecase like mine.

Charlie85270 commented 3 years ago

No news about this issue ?

iamandrewluca commented 3 years ago

My notes on this issue.

This happens only when trying to renderToStaticMarkup(<App />); and inside App there is a useState, useReducer or useMemo that also uses renderToStaticMarkup

e.g.

import React, { useMemo, useReducer, useState } from "react";
import { renderToStaticMarkup } from "react-dom/server";

const hello = <h1>Hello Andrew!</h1>;
const reducer = (state: any) => state;

function App() {
  // Uncomment one of below lines and you will get a error
  // useMemo(() => renderToStaticMarkup(hello), []);
  // useState(() => renderToStaticMarkup(hello));
  // useReducer(reducer, "", () => renderToStaticMarkup(hello));
  return <div />;
}

renderToStaticMarkup(<App />);
TypeError: Cannot set property 'memoizedState' of null

ps: renderToStaticMarkup can be replaced with renderToString, will be same issue ps2: and if using useRef(renderToStaticMarkup(hello)); another error is showed Invalid hook call. Hooks can only be called...

https://codesandbox.io/s/nervous-liskov-ztt4r?file=/src/index.tsx

lauchness commented 3 years ago

I'm having this issue as well using Next.js.

My experience aligns with @iamandrewluca - when using the ReactDOMServer.renderToString() method within a useMemo, I get the error: TypeError: Cannot set property 'memoizedState' of null

ktfth commented 3 years ago

I can work on this task? How is the better aproach to test on code base?

ktfth commented 3 years ago

I made some changes on related file of ReactPartialRendererHooks check it out 21382

ktfth commented 3 years ago

@gaearon Can you help to solve some little details on the pull request process?

SahilSunda commented 3 years ago

Is this issue is resolved. @gaearon can you confirm?? Else if not then I can take it and start from now...

taejs commented 3 years ago

I'll investigate it.

taejs commented 3 years ago

A reason

Look at the code that occurs this error, the ReactPartialRenderer is already initialized to render . https://github.com/facebook/react/issues/16416#issue-481517156. But useMemo Hook calls processLink() -> renderToStaticMarkup() -> another ReactPartialRenderer is initialized. For now, those two ReactPartialRenderers are sharing the environment(closure) of ReactPartialRendererHooks.js

UseMemo - ReactFizzHooks.js

// workInProgressHook: {memoizedState : null}
const nextValue = nextCreate();
//this line calls renderToStaticMarkup() - another ReactPartialRenderer is created.

workInProgressHook.memoizedState = [nextValue, nextDeps]; // workInProgressHook is resetted. (become null)

That's why this error has occurred.


There are some ideas to resolve this issue.

  1. Throw an error when the renderer is created more than one.
  2. Change ReactPartialRenderHooks into class, make each ReactPartialRenderer to have a ReactPartialRenderHooks instance.
  3. Create Weakmap using as a key for Component, a value for the environment. Using this environment, Remove reference of Weakmap when finishHooks() called.

@bvaughn @gaearon If you don't mind, would you share your opinion on which one is the best? Is there a better way to handle this?

b-smets commented 3 years ago

Any updates on this ticket?

eliyamelamed1 commented 3 years ago

Mind if I take this?

SaNsK11 commented 3 years ago

@ambar can I take up this issue if no one is working on it?

Archies13Singh commented 3 years ago

Can I take these issue, if no one is working ?

Lizzyrho21 commented 2 years ago

I can take a crack at this issue if nobody else has. Thanks!

gaearon commented 2 years ago

@Lizzyrho21

There's been a few attempts to fix it but so far they've tried to address the symptoms rather than the root cause. I think it might be easier for me to look into this, but what would be helpful is a PR with a failing test for this issue. Would you like to create one? ReactServerRendering-test.js might be a good place to add it. Run yarn test --watch ReactServerRendering-test and try to make a test that causes this error.

iamandrewluca commented 2 years ago

@gaearon I tried to write a test but it seems it not fails in that environment. Did I miss something 🤔

it('should not crash when used inside useMemo', () => {
  function App() {
    React.useMemo(() => ReactDOMServer.renderToString(<span />), []);
    return <span />;
  }

  expect(() => ReactDOMServer.renderToString(<App />))
    .toThrowError(`Cannot set properties of null (setting 'memoizedState')`)
});
  ● ReactDOMServer › renderToString › should not crash when used inside useMemo

    expect(received).toThrowError(expected)

    Expected substring: "Cannot set properties of null (setting 'memoizedState')"

    Received function did not throw

This piece of code fails in cra browser, cra test, codesandbox and node (using createElement) with same error:

TypeError: Cannot set property 'memoizedState' of null

https://codesandbox.io/s/nervous-liskov-ztt4r?file=/src/index.tsx

image

addemod commented 1 year ago

Still an issue. Is anyone working on this?