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

useTransition - startTransition does not work on React.memo when is SimpleMemoComponent #17318

Closed
salvoravida opened this issue Nov 8, 2019 · 10 comments

Comments

@salvoravida
Copy link

Do you want to request a feature or report a bug?
bug
What is the current behavior?
useTransition - startTransition do not work on React.memo when is SimpleMemoComponent

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:

What is the expected behavior?
startTransition work with SimpleMemoComponent

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

@salvoravida salvoravida changed the title useTransition - startTransition do not work on React.memo when is SimpleMemoComponent useTransition - startTransition does not work on React.memo when is SimpleMemoComponent Nov 8, 2019
@threepointone
Copy link
Contributor

Please make a reproducing example on codesandbox, it’s not clear what you’re asking.

@salvoravida
Copy link
Author

salvoravida commented Nov 10, 2019

isPending not working

Click on "With Transition Start"
OK -> https://codesandbox.io/s/test-redux-in-concurrent-mode-with-react-redux-salvoravidareact-redux-9lun7
KO -> https://codesandbox.io/s/test-redux-in-concurrent-mode-with-react-redux-salvoravidareact-redux-68pbd

only line 85 is changed

const _MyComponent = () => {
  const status = useSelector(s => s.status);

  if (status === STATUS.PENDING) {
    throw promise;
  }
  return null;
};

//_MyComponent.defaultProps = {};
const MyComponent = React.memo(_MyComponent);

the problem maybe is here in react-dom

function updateMemoComponent(current$$1, workInProgress, Component, nextProps, updateExpirationTime, renderExpirationTime) {
  if (current$$1 === null) {
    var type = Component.type;

    if (isSimpleFunctionComponent(type) && Component.compare === null && // SimpleMemoComponent codepath doesn't resolve outer props either.
    Component.defaultProps === undefined) {
      var resolvedType = type;

      {
        resolvedType = resolveFunctionForHotReloading(type);
      } // If this is a plain function component without default props,
      // and with only the default shallow comparison, we upgrade it
      // to a SimpleMemoComponent to allow fast path updates.


      workInProgress.tag = SimpleMemoComponent;
      workInProgress.type = resolvedType;

      {
        validateFunctionComponentInDev(workInProgress, type);
      }

      return updateSimpleMemoComponent(current$$1, workInProgress, resolvedType, nextProps, updateExpirationTime, renderExpirationTime);
    }

@dai-shi
Copy link
Contributor

dai-shi commented Nov 10, 2019

@salvoravida Hi, do you think you can create a smaller reproduction? You don't need redux or even useTransition to reproduce this bug, do you? If what you are observing is correct, this issue is related with #17314.

@stale
Copy link

stale bot commented Feb 8, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Feb 8, 2020
@stale
Copy link

stale bot commented Feb 15, 2020

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!

@stale stale bot closed this as completed Feb 15, 2020
@gaearon gaearon reopened this Feb 15, 2020
@stale stale bot removed the Resolution: Stale Automatically closed due to inactivity label Feb 15, 2020
@gaearon
Copy link
Collaborator

gaearon commented Feb 15, 2020

We haven't checked if this is a bug. It might be.

@dai-shi
Copy link
Contributor

dai-shi commented Feb 15, 2020

@gaearon Probably, this issue and #17314 can be merged into #17028. All of them originated from my code. #17028 has a small reproduction, while this issue is good because it points out the issue only happens with React.memo only when it has no areEqual.

@acdlite
Copy link
Collaborator

acdlite commented Feb 21, 2020

Thanks for the bug report!

I believe this was fixed by #18091

You can confirm using react@0.0.0-experimental-ea6ed3dbb and react-dom@0.0.0-experimental-ea6ed3dbb.

I'm going to close this issue, but if the bug persists please comment and we'll reopen.

@acdlite acdlite closed this as completed Feb 21, 2020
@dai-shi
Copy link
Contributor

dai-shi commented Feb 21, 2020

Confirmed the fix with 0.0.0-experimental-5de5b6150 on my end. Thanks so much!

@salvoravida
Copy link
Author

great! thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants