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

Forward ref to iframe in SandpackPreview #238

Conversation

feychenie
Copy link
Contributor

@feychenie feychenie commented Dec 3, 2021

What kind of change does this pull request introduce?

Improvement:
Adds the possibility to get a ref to the preview iframe on the SandpackPreview component. ie: <SandpackPreview ref={myRefFromUseRef} />

What is the current behavior?

Currently, it is not possible to have a reference to the preview iframe when using the react components, the only way is to use the Sandpack client.

What is the new behavior?

Getting a reference to the iframe allows to use the postMessage API to communicate between the parent window and the app running in the preview.

What steps did you take to test this? This is required before we can merge, make sure to test the flow you've updated.

  1. I tested that the internal ref used in the preview component still works as it did before
  2. I tested that I have a working ref on <SandpackPreview ref={myRefFromUseRef} />

Checklist

  • Documentation N/A
  • Ready to be merged

Comments

I figure that you might have an official way to communicate between the preview and the parent in your roadmap and that you might be wary about exposing this in the meantime, as you are using postMessage already for internals and there is a risk of reserved message clashes.
I've lost quite some hairs on the subject at GraphCMS (and still am) for our UI extensions feature, so feel free to DM me on Discord about this PR or this subject in general, happy to contribute here :)

@seflless
Copy link

seflless commented Dec 3, 2021

Looks good to me. Not my call obviously. What use cases do you have? Would be curious to add weight to prioritizing merging this.

For me it’s that I’m making a 3D renderer game engine and I need the iframe sending back its rendering output to the parent frame for being mixed into other visuals

@CompuIves CompuIves requested review from alexnm and danilowoz December 4, 2021 10:20
@danilowoz
Copy link
Member

Hey @feychenie, thanks for your contribution!
Indeed, Sandpack was missing an official way to communicate with the iframe and the client. Your proposal looks good, and it seems the straightforward way to access the iframe context. However, I would like to provide a way to interact with all clients/iframe under the same SandpackContext, and not only give access to a single one.

That's why in #241 I proposed to expose the clients in the Sandpack context, which gives you the possibility to pass messages to the iframe but also ensure you're doing so for all of them in the same context.

Let's me know your thoughts

@feychenie
Copy link
Contributor Author

Thanks @danilowoz for the feedback!
The solution proposed in #241 looks way better indeed, hope it gets released soon 💯

Closing this one then 👍🏻

@feychenie feychenie closed this Dec 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants