-
Notifications
You must be signed in to change notification settings - Fork 7.8k
[beta] PoC: replace Sandpack with react-runner #4339
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
Conversation
Size Changes📦 Next.js Bundle AnalysisThis analysis was generated by the next.js bundle analysis action 🤖 🎉 Global Bundle Size Decreased
DetailsThe global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster. Any third party scripts you have added directly to your app using the If you want further insight into what is behind the changes, give @next/bundle-analyzer a try! One Hundred Eighty-five Pages Changed SizeThe following pages changed size from the code in this PR compared to its base branch:
DetailsOnly the gzipped size is provided here based on an expert tip. First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If Any third party scripts you have added directly to your app using the Next to the size is how much the size has increased or decreased compared with the base branch of this PR. If this percentage has increased by 10% or more, there will be a red status indicator applied, indicating that special attention should be given to this. |
831f51c
to
c877e54
Compare
I think this looks very promising. How much work is it to get this styled so that the editor works and we can compare 1:1? I see some sandboxes on https://beta-reactjs-org-n2pucbumf-fbopensource.vercel.app/learn/updating-objects-in-state are a bit borked. |
@@ -0,0 +1,117 @@ | |||
import React, {useState, useMemo} from 'react'; | |||
import {Runner, importCode, Scope} from 'react-runner'; | |||
import * as UseImmer from 'use-immer'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not super great that we have to load everything we use in every sandbox. We might want to have "heavier" examples in the future with third party libs. They shouldn't bloat the initial editor. I guess having a list of them here is probably fine though. Maybe as dynamic imports and somehow lazy-loaded?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes lazy loading should definitely work, and we can still keep the package.json
file and analyse the dependency, right now I just ignored that file
@gaearon There are some visual regressions, as previously we target the to root document but now we use a shadow DOM instead, I think it should be super easy to fix, if we decided to make the switch |
I don't think we can decide whether to make a switch until we have a good idea of how it works end-to-end, including all the styling, having the code highlighting hooked up, etc. I know it's a bunch of work but it's hard to compare otherwise. |
@gaearon I totally understand, I'll try to fix the styling issues first, it works well in my demo though, and the benefit is obvious, the demo is part of our app, so it's super fast, and can reflect user's input with no delay, no bundling, only compiling the JSX syntax |
c877e54
to
c2dc514
Compare
c2dc514
to
fe32d26
Compare
@gaearon tweaked the styling a bit, though there are still a lot of work to do |
A few quick thoughts. Overall approachI think it's pretty clear this is the right direction for the tradeoffs we have today. I was hoping we can introduce a mode like this to Sandpack in longer term. The instant feedback is hard to beat. The network waterfalls and other inefficiencies in Sandpack today are unfortunate because our use case is very limited. I think it's plausible the Sandpack package will evolve to support this model as opt-in. I'd like to hear more from their perspective and I'm not saying we should necessarily switch packages to yours. But it seems clear an approach like this is promising, whether through this package or something else. Production vs DevelopmentOne thing that seems unfortunate with this approach is that errors are minified.
But since we're on the docs site, it seems confusing. It also means we'd miss One option would be to make the docs site use a development build of React. Seems bad. Another option would be to load development build of React, but just for sandboxes. Seems okay-ish. Though also feels a bit silly if we just want to render a few examples. I'm not sure about the best approach. There could be some middle ground. Like use prod build until dev builds loads lazily. Though might be confusing if a sandbox has an error initially. Fast RefreshIt's not great that sandboxes lose state on edits. Sandpack implements a simple Fast Refresh integration to avoid that. I think you can't do this very easily because you don't have Babel though and can't run the Fast Refresh transform. Maybe some simplified version could be written. It's thorny with multi-file edits though. Maybe it's ok that it doesn't work. But feels like a regression. Server ComponentsWe'll likely have some movement towards server-driven examples in the future. Like showcasing Server Components. That may require a Node.js environment (like the emulation StackBlitz is doing). But that's really heavy. Alternatively, it may be approximated with a Worker environment. We were thinking this would be the likely "local debugging" story for Server Components anyway. From this perspective, Sandpack might already have some infrastructure that would be easier to reuse than to build. But I don't know about this. It's just something else to keep in mind. Third Party Libs/TransformsIt's plausible we'd want to show some other third party libraries. Or something that requires transforms. E.g. if we ever have an official CSS-in-JS solution. Then if it's on top of Babel, we need some way to run it. So I wonder how avoidable Babel is in long term anyway. |
@gaearon Thanks for your thorough feedbacks, here are my rough answers:
One possible solution is map error message back to decoded one, since the data will be stored in the same app, I think it should not be that hard
I haven't tested yet but the transpiler React Runner uses is
This website it built with Next.js which also supports Server Components, probably we can just use it? Or for this case we can just other solutions like Sandpack or CodeSandbox or StackBlitz, we are not bonded to use only one tool for all scenarios
If we do need babel plugins, we can use
I'm not sure about that, as we have totally different direction, Sandpack is framework agnostic, and provides a full dev environment, while React Runner is designed for React only, it's super lightweight and fast, and it will be integrated to your app as a part of it, so it's perfect if you only want to demo your own components, React Docs is a bit complicated here as we also want some dev environment experience BTW I also made some improvements on the editor comparing to Sandpack's editor(we both use CodeMirror6), e.g. your editing history will be kept on switching files, I only spent some spare time on that, I believe with wider adoption we can make it better together. |
fe32d26
to
87a6a5d
Compare
Closing as per comment in the issue, but thank you very much, @nihgwu for taking the time to contribute! 👍 |
PoC for #4338
Preview
@gaearon It's quite rough but should work already, haven't checked why theme is not applied for CodeMirror but it works here