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

Feature request for React Hooks: Handle Spaces initialization in SpacesProvider #265

Open
mms-uret opened this issue Nov 28, 2023 · 4 comments
Labels
enhancement New feature or request

Comments

@mms-uret
Copy link

mms-uret commented Nov 28, 2023

When the <SpacesProvider> React component is used, most of the time, the spaces object needs to be created first.

From the documentation:

const ably = new Realtime.Promise({ key: "VS4Rww.ijkxYg:*****", clientId: 'clemons' });
const spaces = new Spaces(ably);

root.render(
  <SpacesProvider client={spaces}>
    <SpaceProvider name="my-space">
      <App />
    </SpaceProvider>
  </SpacesProvider>
)

It would be nice if, alternatively to the spaces object, the key and the clientId could be passed to the SpacesProvider and the SpacesProvider cares about initializing Ably and Spaces:

  <SpacesProvider key="VS4Rww.ijkxYg:*****" clientId="clemons">
    <SpaceProvider name="my-space">
      <App />
    </SpaceProvider>
  </SpacesProvider>

Or alternatively have an object, which is passed to Realtime.Promise().

This would save some boilerplate code and also would make sure that implementers don't have to remember to put the Spaces initialization into a useMemo()/ useEffect() hook ;)

@dpiatek
Copy link
Contributor

dpiatek commented Nov 28, 2023

hey @mms-uret thanks for the suggestion. Did you run into issues with:

const ably = new Realtime.Promise({ key: "VS4Rww.ijkxYg:*****", clientId: 'clemons' });
const spaces = new Spaces(ably);

being called too often? I'll forward your proposal to the product team. An alternative way to share the client is also using the AblyProvider https://ably.com/docs/getting-started/react#ably-provider, did you try that?

@pchr-srf
Copy link

pchr-srf commented Nov 28, 2023

Hey @dpiatek ! Yes, we actually raised a support ticket with Ably because we couldn't figure out our own mistake. The solution was pretty simple:

-  const ably = new Realtime.Promise({ key: ablyKey, clientId: user.email });
-  const spaces = new Spaces(ably);
+  const spaces = useMemo(() => {
+    const ably = new Realtime.Promise({ key: ablyKey, clientId: user.email });
+    return new Spaces(ably);
+  }, [ablyKey, user.email]);

(we're using the SpacesProvider in a component)

@mms-uret
Copy link
Author

mms-uret commented Nov 28, 2023

Ah, I didn't know about <AblyProvider>. Then maybe it just needs a way to better integrate AblyProvider and SpacesProvider:

<AblyProvider>
  <SpacesProvider>
    <SpaceProvider>
      <App/>
    </SpaceProvider>
  </SpacesProvider>
</AblyProvider>

There is still something needed to initialize the Spaces instance, I think....
Or maybe integrate AblyProvider into SpacesProvider since Spaces cannot be used without Ably?

@dpiatek
Copy link
Contributor

dpiatek commented Nov 28, 2023

@mms-uret you can see how we use the providers in our examples repo https://github.com/ably-labs/realtime-examples/blob/main/examples/vite-component-locking/src/index.tsx. It's useful however to get your feedback here.

The reason the providers are separated is to ensure compatibility with existing apps which already initialise client and future Ably libraries that could share the connection/client. There might be a case, however, as you point out, to just combine that into one component.

@dpiatek dpiatek added the enhancement New feature or request label Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

No branches or pull requests

3 participants