Open gabrielmfern opened 3 weeks ago
vercel[bot]
commented
3 weeks ago The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Comments | Updated (UTC) |
|---|---|---|---|---|
| react-email | ✅ Ready (Inspect) | Visit Preview | 💬 Add feedback | May 17, 2024 7:22pm |
| react-email-demo | ✅ Ready (Inspect) | Visit Preview | 💬 Add feedback | May 17, 2024 7:22pm |
devklepacki
commented
3 weeks ago Can't wait for this, currently HMR is far from perfect. The worst thing is that it's slow, but sometimes also don't reload at all 🫣 So one have to wait, wait, wait... only to realize that the email won't refresh and then reload manually.
Btw. HMR is also scrolling to top each time for longer emails, probably due to the change of iframe contents. Was preventing this discussed or is included in these changes?
gabrielmfern
commented
3 weeks ago Hey @devklepacki!
The worst thing is that it's slow
Yeah, that's true, but this PR doesn't address that. We do have a few changes we are working on for the 3.0 that should get this in a much better shape. It's going to be muuuch faster as well.
Was preventing this discussed or is included in these changes?
It was something we noticed, but I'm not sure if this can be avoided, unless we render the email template some other way where only the modified parts actually get rendered back into the iframe
devklepacki
commented
3 weeks ago Yeah, that's true, but this PR doesn't address that.
I see. For now, just knowing that it will eventually refresh is also great.
It was something we noticed, but I'm not sure if this can be avoided unless we render the email template some other way where only the modified parts actually get rendered back into the iframe
Maybe "just" watching the scroll position and trying to restore it after the iframe content is updated would be enough? Of course, it's a "just" because it always sounds easy, but not always is.
gabrielmfern
commented
3 weeks ago Maybe "just" watching the scroll position and trying to restore it after the iframe content is updated would be enough? Of course, it's a "just" because it always sounds easy, but not always is.
It's always more complicated 😅. I tried experimenting a bit yesterday, and I think there's a way if we just use a shadow DOM instead of an iframe, and render the HTML using react-dom instead as well. PR might be coming for this sooner
Background
A bit of time ago I got to implementing an improvement to the preview server that made it hot reload when there were changes made to the dependencies.
I did this through a sort of dependency graph that connects each module to its dependents and to its dependencies. This was very useful because it made it so that it was both easy to update the dependency graph in an optimal way, while still being easy to resolve through the dependents and notify the Next app that they changed.
What this fixes
The initial implementation I made though, presented a few issues. Not necessarily in the way I did it, but a few bugs have arisen as mentioned from https://github.com/resend/react-email/issues/1366#issuecomment-2042762918 onwards.
The bugs were, in summary:
indexfilesThese, were, in turn, caused by:
export * from 'my-module')This PR fixes those such that the issues have gone away.
How can we test to make sure it is fixed?
I'm waiting on @sorokin-energi to open an issue on this with a minimal reproduction so that we can get to testing it on their use case.
I have done a few tests myself using some of their example file structures, and they are working as expected.