shellscape / jsx-email

Build emails with a delightful DX
https://jsx.email
MIT License
991 stars 33 forks source link

Can't run a script with ts-node that calls the render function #78

Closed co-sic closed 10 months ago

co-sic commented 10 months ago

Expected Behavior

I should be able to run the script

Actual Behavior

I get an error:

var import_shikiji = require("shikiji");
                     ^
Error [ERR_REQUIRE_ESM]: require() of ES Module /home/[*****]/node-playground/node_modules/shikiji/dist/index.mjs not supported.
Instead change the require of /home/[*****]/node-playground/node_modules/shikiji/dist/index.mjs to a dynamic import() which is available in all CommonJS modules.
    at Object.<anonymous> (/home/[*****]/node-playground/node_modules/@jsx-email/code/dist/index.js:42:22)
    at require.extensions.<computed> [as .js] (/home/[*****]/node-playground/node_modules/ts-node/dist/index.js:851:20)
    at Object.<anonymous> (/home/[*****]/node-playground/node_modules/@jsx-email/all/dist/index.js:22:25)
    at require.extensions.<computed> [as .js] (/home/[*****]/node-playground/node_modules/ts-node/dist/index.js:851:20)
    at Object.<anonymous> (/home/[*****]/node-playground/src/templates/template.tsx:8:15)
    at m._compile (/home/[*****]/node-playground/node_modules/ts-node/dist/index.js:857:29)
    at require.extensions.<computed> [as .tsx] (/home/[*****]/node-playground/node_modules/ts-node/dist/index.js:859:16)
    at Object.<anonymous> (/home/[*****]/node-playground/src/script.tsx:7:20)
    at m._compile (/home/[*****]/node-playground/node_modules/ts-node/dist/index.js:857:29)
    at require.extensions.<computed> [as .tsx] (/home/[*****]/node-playground/node_modules/ts-node/dist/index.js:859:16)
    at phase4 (/home/[*****]/node-playground/node_modules/ts-node/dist/bin.js:466:20)
    at bootstrap (/home/[*****]/node-playground/node_modules/ts-node/dist/bin.js:54:12)
    at main (/home/[*****]/node-playground/node_modules/ts-node/dist/bin.js:33:12)
    at Object.<anonymous> (/home/[*****]/node-playground/node_modules/ts-node/dist/bin.js:579:5) {
  code: 'ERR_REQUIRE_ESM'
}
error Command failed with exit code 1.

Additional Information

It runs no problem with v2.0.0 of jsx-email/all

co-sic commented 10 months ago

What am i missing of the issue template? I included a minimal reproduction repository link.

shellscape commented 10 months ago

yeah I originally dropped that comment from reading on my phone and for some reason the link did not render, I deleted that reply 😅

co-sic commented 10 months ago

Okay :sweat_smile:

shellscape commented 10 months ago

I was able to reproduce it on my end using your repo.

ts-node src/script.tsx does indeed fail, but npx email build succeeds.

→ npx email build src/templates/template.tsx 
Found 1 files:
   src/templates/template.tsx

Starting build...

Build complete. File(s) written to: /code/forks/node-playground/.rendered

And (with --pretty)...

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html lang="en" dir="ltr">

  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>

  <body>
    <table align="center" width="100%" role="presentation" cellspacing="0" cellpadding="0" border="0" style="max-width:37.5em">
      <tbody>
        <tr style="width:100%">
          <td>Hello</td>
        </tr>
      </tbody>
    </table>
  </body>

</html>

My hunch is that it's a ts-node problem, because there's no reason it would work in the email binary (which is shipped as CJS) and not using render directly, because that's exactly what the CLI does. I'll dig on this some more, but in the mean time, the CLI will get you going.

co-sic commented 10 months ago

Thanks for looking into this and the fast reply!

I pushed a commit where i added a new command. It has nothing to do with ts-node, since it also fails if i run build and then run the compiled file with node directly. I have a setup where i publish to packages, one which is dynamic and includes the render function of jsx-email, and one that is auto generated out of the templates where i just publish the generated html with placeholders wraped in a function, so it has no dependencies (and way better performance). So i kind of need to run the render function with node or ts-node to build this.

EDIT: i guess i could also use email build maybe to generate the html i need for my static package, but this would mean a lot of refactoring in the current setup i have, so i rather stay on v2 until this hopefully works again

shellscape commented 10 months ago

You could always try tsx, it works without issue.

since it also fails if i run build and then run the compiled file with node directly

Well it's definitely a node resolution problem. The CLI uses render just how you're using it in the script, and it works.

A better workaround, though more leg work, is to install all of the packages separately, since it seems to also be an all problem with exports (this is not the first problem with all):

// template
import { Body } from "@jsx-email/body";
import { Container } from "@jsx-email/container";
import { Head } from "@jsx-email/head";
import { Html } from "@jsx-email/html";
// script
import { Template } from "./templates/template";
import { render } from "@jsx-email/render";

The billion-packages setup is a holdover from when we forked from react-email and it's never made sense. We just kept it for immediate compat and to push the project forward. This PR https://github.com/shellscape/jsx-email/pulls is going to change all of that, and these bizarre import and export resolution errors should vanish.

co-sic commented 10 months ago

I also thought that was crazy with all the single packages from react-email ... Do you have a rough estimation when you are releasing the mentioned PR? If this will fix the problem then i think i will just wait for that, or maybe use the single packages until it is released. Thanks for that tip! : )

shellscape commented 10 months ago

My guess would be by end of week. We'll see how work and kids and stuff plays out. I'd recommend the individual packages approach bc it's newer stuff and capability that will be in the monopackage.

co-sic commented 10 months ago

@shellscape i updated everything to individual packages and it seems to work (i can run the script), but my last problem is now that my existing tests with jest are no longer working. I updated my example repo with a simple test case: https://github.com/co-sic/node-playground Seems to be related to this, since this throws an error: You need to run with a version of node that supports ES Modules in the VM API. In my main repository i actually get a differnt error: A jest worker process (pid=1178056) was terminated by another process: signal=SIGSEGV, exitCode=null. Operating system logs may contain more information on why this occurred. and i could not find any solution to this. Maybe this is a jest problem, but with v2 of the render function it was working

shellscape commented 10 months ago

Jest is well-known to do weird things under the covers, like futzing with require to enable import mocks. This https://github.com/jestjs/jest/issues/10944 looks like the same error you're running into. I'd highly recommend ditching Jest for Vitest (that's what we use here), but that might be too big a lift.

My efforts on the single package are getting closer to done, and you can try that out by installing jsx-email@next right now. However, I am using dynamic imports (e.g. await import(...)) there too. If Jest has issues with that, I'm not sure there's anything that can be done.

co-sic commented 10 months ago

Thx for recommending vitest, its working without any problems. Since i have all the templates in a dedicated package it is no problem to switch from jest :)

shellscape commented 10 months ago

Awesome!

co-sic commented 10 months ago

@shellscape i just updated to the new single package release and i now again get the error i posted in the original post here. I updated the playground example i linked in the orginal post.

shellscape commented 10 months ago

Have you tried tsx?

co-sic commented 10 months ago

Sorry forgot to test that. It works, so that fine for me :+1:

shellscape commented 10 months ago

Awesome. We'll add this to the FAQ on the docs