Skip to content
This repository was archived by the owner on Jan 1, 2025. It is now read-only.

[SSR] Resolve promise on server side rendering as endBatch is only called on useEffect#2073

Closed
tirthbodawala wants to merge 3 commits into
facebookexperimental:mainfrom
Atyantik:main
Closed

[SSR] Resolve promise on server side rendering as endBatch is only called on useEffect#2073
tirthbodawala wants to merge 3 commits into
facebookexperimental:mainfrom
Atyantik:main

Conversation

@tirthbodawala

Copy link
Copy Markdown
Contributor

Reference Issue: #1960

The function sendEndOfBatchNotifications is only executed on useEffect as below:

useEffect(() => {
    // enqueueExecution runs this function immediately; it is only used to
    // manipulate the order of useEffects during tests, since React seems to
    // call useEffect in an unpredictable order sometimes.
    Queue.enqueueExecution('Batcher', () => {
      endBatch(storeRef.current);
    });
  });

This caused the SSR usage of async selector to get stuck in infinite loading.

With this PR, we make sure that when the original promise is resolved, we resolve the handleLoadable promise as well. Thus making it compatible with SSR rendering

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 20, 2022
@tirthbodawala

Copy link
Copy Markdown
Contributor Author

Hi Team,
I have also added the catch block in case the promise fails on SSR, thus making sure that the returned promise is properly rejected and the ErrorBoundary can handle it.

@tirthbodawala

Copy link
Copy Markdown
Contributor Author

@wd-fb @drarmstr please review and let me know your thoughts and improvements in this PR

if (isSSR && isPromise(loadable.contents)) {
loadable
.contents
.then((d) => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use .finally() instead?

@drarmstr drarmstr changed the title #1960 Resolve promise on server side rendering as endBatch is only called on useEffect [SSR] Resolve promise on server side rendering as endBatch is only called on useEffect Nov 2, 2022
@facebook-github-bot

Copy link
Copy Markdown
Contributor

@drarmstr has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@tirthbodawala

tirthbodawala commented Nov 2, 2022 via email

Copy link
Copy Markdown
Contributor Author

const recoverableViolation = require('recoil-shared/util/Recoil_recoverableViolation');
const useComponentName = require('recoil-shared/util/Recoil_useComponentName');
const {isSSR} = require('recoil-shared/util/Recoil_Environment');
const {isPromise} = require('recoil-shared/util/Recoil_isPromise');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, this shouldn't work as-is since Recoil_isPromise.js exports the function directly. Is this the version you tested with?

I updated the diff to fix this import and use finally() instead, though unfortunately the changes won't sync back to this PR. I'll test with internal unit tests but we don't have a good SSR test so please try it out after it merges?

drarmstr pushed a commit to drarmstr/Recoil that referenced this pull request Nov 8, 2022
…n useEffect (facebookexperimental#2073)

Summary:
Reference Issue: [https://github.com/facebookexperimental/Recoil/issues/1960](https://github.com/facebookexperimental/Recoil/issues/1960)

The function sendEndOfBatchNotifications is only executed on useEffect as below:
```
useEffect(() => {
    // enqueueExecution runs this function immediately; it is only used to
    // manipulate the order of useEffects during tests, since React seems to
    // call useEffect in an unpredictable order sometimes.
    Queue.enqueueExecution('Batcher', () => {
      endBatch(storeRef.current);
    });
  });
```

This caused the SSR usage of async selector to get stuck in infinite loading.

With this PR, we make sure that when the original promise is resolved, we resolve the `handleLoadable` promise as well. Thus making it compatible with SSR rendering

Pull Request resolved: facebookexperimental#2073

Differential Revision: https://www.internalfb.com/diff/D40948099?entry_point=27

Pulled By: drarmstr

fbshipit-source-id: 9ac77a898ecd118005d87ddf0a461478cf498488
drarmstr pushed a commit to drarmstr/Recoil that referenced this pull request Nov 8, 2022
…n useEffect (facebookexperimental#2073)

Summary:
Reference Issue: [https://github.com/facebookexperimental/Recoil/issues/1960](https://github.com/facebookexperimental/Recoil/issues/1960)

The function sendEndOfBatchNotifications is only executed on useEffect as below:
```
useEffect(() => {
    // enqueueExecution runs this function immediately; it is only used to
    // manipulate the order of useEffects during tests, since React seems to
    // call useEffect in an unpredictable order sometimes.
    Queue.enqueueExecution('Batcher', () => {
      endBatch(storeRef.current);
    });
  });
```

This caused the SSR usage of async selector to get stuck in infinite loading.

With this PR, we make sure that when the original promise is resolved, we resolve the `handleLoadable` promise as well. Thus making it compatible with SSR rendering

Pull Request resolved: facebookexperimental#2073

Differential Revision: https://www.internalfb.com/diff/D40948099?entry_point=27

Pulled By: drarmstr

fbshipit-source-id: 30331664b0d0e8c6b05ac46a0d9bedccd34e4982
@tirthbodawala

Copy link
Copy Markdown
Contributor Author

@drarmstr a fix related to this integrated in any version that I can pull and test?

snipershooter0701 pushed a commit to snipershooter0701/Recoil that referenced this pull request Mar 5, 2023
…n useEffect (#2073)

Summary:
Reference Issue: [https://github.com/facebookexperimental/Recoil/issues/1960](https://github.com/facebookexperimental/Recoil/issues/1960)

The function sendEndOfBatchNotifications is only executed on useEffect as below:
```
useEffect(() => {
    // enqueueExecution runs this function immediately; it is only used to
    // manipulate the order of useEffects during tests, since React seems to
    // call useEffect in an unpredictable order sometimes.
    Queue.enqueueExecution('Batcher', () => {
      endBatch(storeRef.current);
    });
  });
```

This caused the SSR usage of async selector to get stuck in infinite loading.

With this PR, we make sure that when the original promise is resolved, we resolve the `handleLoadable` promise as well. Thus making it compatible with SSR rendering

Pull Request resolved: facebookexperimental/Recoil#2073

Reviewed By: wd-fb

Differential Revision: D40948099

Pulled By: drarmstr

fbshipit-source-id: 0a2c89972fc8a841b8271718bf8f212153ab3ce5
@vine1993

vine1993 commented Apr 8, 2024

Copy link
Copy Markdown

im trying to use the recoil version 0.77 with nextjs14 and I'm still getting the same timeout error

@tirthbodawala .

import { ReactNode, Suspense } from "react";

import { RecoilContext } from "./recoil";
import StyledJsxRegistry from "./registry";
import { LayoutTheme } from "./layoutTheme";


export default function RootLayout({
  children,
}: {
  children: ReactNode;
}) {
  return (
    <html lang="en">
      <body>
        <RecoilContext>
          <StyledJsxRegistry>
            <Suspense fallback={"CARREGANDO AE"}>
              <LayoutTheme>
                {children}
              </LayoutTheme>
            </Suspense>
          </StyledJsxRegistry>
        </RecoilContext>
    </body>
    </html>
  );
}
export function LayoutTheme({ children }: { children: React.ReactNode }) {
    //@ts-ignore
    const tenant = useRecoilValue<typeof TENANT>(tenantState)
 
    return (
        <>
            <style jsx global>{`
                :root {${tenant.tailwind_theme}}
            `}
            </style>
            {children}
        </>
    )
}
export const tenantState = selector({
    key: 'tenantState',
    get: async () => {
        return await new Promise(resolve => setTimeout(()=> resolve(TENANT2),2000))
    }
})

eagle2722 added a commit to eagle2722/Recoil that referenced this pull request Sep 21, 2024
…n useEffect (#2073)

Summary:
Reference Issue: [https://github.com/facebookexperimental/Recoil/issues/1960](https://github.com/facebookexperimental/Recoil/issues/1960)

The function sendEndOfBatchNotifications is only executed on useEffect as below:
```
useEffect(() => {
    // enqueueExecution runs this function immediately; it is only used to
    // manipulate the order of useEffects during tests, since React seems to
    // call useEffect in an unpredictable order sometimes.
    Queue.enqueueExecution('Batcher', () => {
      endBatch(storeRef.current);
    });
  });
```

This caused the SSR usage of async selector to get stuck in infinite loading.

With this PR, we make sure that when the original promise is resolved, we resolve the `handleLoadable` promise as well. Thus making it compatible with SSR rendering

Pull Request resolved: facebookexperimental/Recoil#2073

Reviewed By: wd-fb

Differential Revision: D40948099

Pulled By: drarmstr

fbshipit-source-id: 0a2c89972fc8a841b8271718bf8f212153ab3ce5
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. server rendering

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants