resendlabs
Dynamic import in @react-email/render breaks when using sentry
Describe the Bug
@gabrielmfern This is a follow up from my closed PR (https://github.com/resend/react-email/pull/2455) - I finally had the time to investigate the issue.
The recentish change to dynamically import react-dom in @react-email/render (https://github.com/resend/react-email/blob/canary/packages/render/src/node/render.tsx#L9) is problematic as dynamic imports do not work in workers as per the esm spec (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/import)
IMO its a pretty severe issue, as its not always clear whether workers are even being used in an application.
In my particular case after a lot of investigation I discovered that @sentry/node wraps monitored services in workers (I'm guessing to get better access to debugging/process information, but honestly I'm not exactly sure why) - meaning that any node app running sentry cannot call @react-email/render
@gabrielmfern - you mentioned that the dynamic import was added to handle some nextjs issues - I'm not clear on what they are or if there are any other solutions, but if not, maybe it makes sense to export 2 seperate versions of render, one with dynamic imports and one without?
Which package is affected (leave empty if unsure)
@react-email/render
To Reproduce
You'll need to setup a sentry account to reproduce the issue. Otherwise you may also be able to trigger it by manually calling render inside a node worker.
Expected Behavior
For node not to crash.
The error message for reference is:
TypeError [Error]: Cannot read properties of undefined (reading 'resolve')
at MessagePort.handleMessage (node:internal/modules/esm/worker:179:24)
at [nodejs.internal.kHybridDispatch] (node:internal/event_target:827:20)
at MessagePort.<anonymous> (node:internal/per_context/messageport:23:28)
What's your node version? (if relevant)
22
maybe it makes sense to export 2 seperate versions of render, one with dynamic imports and one without
How would we export them? Like a different export condition? Is there an export condition for workers like that?
You can specify multiple exports in package.json under exports (@react-email/render already does this for esm/cjs etc). If you add a new named export such as ./dynamic-import, you can then manually specify the version you want to import with @react-email/render/dynamic-import. In fact the offending dynamic import react-dom/server uses exactly this pattern.
Because it might be slightly tedious to add a new named export for all the existing targets that @react-email/render already exports and it makes the api/consuming a little more complicated - I personally think that switching back to a static import would be the better option, but I'm not across the reasons for using a dynamic import in next so 🤷
We can test it again to see if Next.js without the dynamic import still causes issues, but if I'm being honest Next.js is just hell to support properly, and the stuff we have to do to support it causes some issues on other places, like yours.
Maybe Next.js has a specific export condition that it supports that we can use to have the dynamic import in, and then the default would be just a norma import
Adding a seperate import for next and having the default use a static import makes sense to me.
Also i can see how next could be tricky as next could be running in the browser, or as a server side node process, but like i said before I’m not across the reason for the dynamic import in next, so i can’t really be of any help there!
@gabrielmfern - just checking in again to see if there was any updates?