[Bug?]: Redwood Studio can't load email templates (SWC) about redwood HOT 18 CLOSED

Thanks for providing such detail @raph90!

Sorry I haven't gotten to this sooner. My initial reaction to your points are:

1. Can all those platform specific dependencies go in as optional dependencies or are they already treated like that? I'll need to reread the SWC docs on this.
2. I'll have to pull out astexplorer.net to examine what I messed up with the parameter extraction.

If you'd like to make a PR for this then I'd be more than happy to help out with it and review it. I do hope to spend some time on this tomorrow and if I get to the bottom of it I'll likely make a PR. I can leave it and give you a few days to tackle this one too if you'd like but if you could let me know in the next day then that would be great.

Thanks again for the details it's really great when people take the time to document things as clearly as you have, awesome!

from redwood.

Hi @Josh-Walker-GM, I'm afraid I don't think I'd be able to get round to a PR for this at the moment - but very happy to talk through stuff here, since I would really love to start contributing to RW.

1. Can all those platform specific dependencies go in as optional dependencies or are they already treated like that? - Yes I think they can, the SO post where I found that solution used npm -optional (I think) and I wasn't sure of the yarn equivalent

2. Yeah I'm not sure what the issue is here - I'm pretty sure I did everything correctly / downloaded the correct packages.

As an additional question - if you wanted to debug / breakpoint through Studio, what would be the correct way to do that? Would you have a launch.json for VS Code that you might be able to share? Or is just logging the way do you think?

I tried loading up redwood and doing the RWFW command on my project, but I wasn't able to get any breakpoints to catch in the api/mail code.

No worries about the write up, many thanks for looking into this!

from redwood.

That's perfect @raph90. I'll find some time to review this again today or tomorrow. We can have this merged soon for sure!

For some context, the studio package isn't representative of the rest of the framework. I quickly created it as an experimental feature so it wasn't built to the same standard as the rest of our packages. Over the new year we'll be transitioning the studio to its own repository built with redwood itself. This should fix these sorts of configuration issues so I'd feel bad making anyone else sink lots of time into fixing these chores.

from redwood.

Assigning this one to you @Josh-Walker-GM, because mailer! 🙂

from redwood.

Hi @Josh-Walker-GM, this is what I've worked out so far.

The SWC issue relates to dev dependencies, it can be fixed by installing the following as dev dependencies:

"@swc/core-darwin-arm64": "1.3.60",
    "@swc/core-darwin-x64": "1.3.60",
    "@swc/core-linux-arm64-gnu": "1.3.60",
    "@swc/core-linux-arm64-musl": "1.3.60",
    "@swc/core-linux-x64-gnu": "1.3.60",
    "@swc/core-linux-x64-musl": "1.3.60",
    "@swc/core-win32-arm64-msvc": "1.3.60",
    "@swc/core-win32-ia32-msvc": "1.3.60",
    "@swc/core-win32-x64-msvc": "1.3.60",

However, there's then an issue with this line (216 in the compiled index.js from @redwoodjs/studio/dist/api/mail/index.js)
const hasParams = exportedComponents[i].declaration.params.length > 0;
params is actually undefined on the object that comes back from the AST generated by SWC - I'm pretty sure this isn't a versioning issue (I made sure all my versions were the same as the ones defined in redwood), so I'm not quite sure how to proceed with it.

If I make params optional:
const hasParams = exportedComponents[i].declaration.params?.length > 0; then the errors go away, but the preview doesn't work for emails when I export them like this:

import {
  Html,
  Text,
  Hr,
  Body,
  Head,
  Tailwind,
  Preview,
  Container,
  Heading,
} from '@react-email/components'

type ExampleEmailProps = {
  name: string
}

export const WelcomeEmail = (props: ExampleEmailProps) => {
  return (
    <Html lang="en">
      <Head />
      <Preview>An example email</Preview>
      <Tailwind>
        <Body className="mx-auto my-auto bg-white font-sans">
          <Container className="mx-auto my-[40px] rounded border border-solid border-gray-200 p-[20px]">
            <Heading className="mx-0 my-[30px] p-0 text-center text-[24px] font-normal text-black">
              Example Email
            </Heading>
            <Text className="text-[14px] leading-[24px] text-black">
              This is an example email which you can customise to your needs.
            </Text>
            <Hr className="mx-0 my-[26px] w-full border border-solid border-[#eaeaea]" />
            <Text className="text-[12px] leading-[24px] text-[#666666]">
              {props.name}
            </Text>
          </Container>
        </Body>
      </Tailwind>
    </Html>
  )
}

and use them like this:

export const sendWelcomeEmail = async (
  recipient: string,
) => {
  await mailer.send(WelcomeEmail({ name: 'hi' }), {
    to: recipient,
    subject: 'New Email',
    from: '[email protected]'
  })
}

In the preview in Redwood studio I get 'component is not a Function'...

Lmk if you'd like to try to work through this one together. Thanks!

from redwood.

@Josh-Walker-GM I'm going to try and have a go at this this evening, I'll let you know how I get on

from redwood.

Okay perfect!

I couldn't reproduce the issue with the dependencies but I do think we should add them to the optional dependencies as discussed above.

I also noticed that I left comments in the original code about what "style" of email component code we support in the ast parsing. Looks like those point to why you saw the "component is not a function" error.

from redwood.

@Josh-Walker-GM here's a PR - let me know what you think, trying things my end and it does seem to work...

#9639

EDIT
I've also just seen your comments 🤦

  // TODO: Support `const X = () => {}; export default X;`
  // TODO: Support `export default function X () => {}`
  // TODO: Support `export default () => {}`

Maybe you'd like me to add those to this PR? It would be a bit more work - an alternative is I also just add a line to the docs saying which syntaxes are supported (after spending a little bit of time working with ASTs, that would be my preference 😆

EDIT AGAIN - It's not the end of the world, I'll add support for those other syntaxes.

from redwood.

Hi @Josh-Walker-GM , I'm still working on this but I'm a bit worried about whatever implementation I propose, because we might be seeing different AST syntax. For example, for your export function code (which works), you have:

try {
        switch (param.pat.type) {
          case 'ObjectPattern':
            propsTemplate = `{${param.pat.properties
              .map((p: any) => {
                return `\n  "${p.key.value}": ?`
              })
              .join(',')}\n}`
            break
        }
      } catch (_error) {
        // ignore for now
      }

If I console log param there, it looks like this:

{
  type: 'Parameter',
  span: { start: 198, end: 222, ctxt: 0 },
  decorators: [],
  pat: {
    type: 'Identifier',
    span: { start: 198, end: 222, ctxt: 3 },
    value: 'props',
    optional: false,
    typeAnnotation: {
      type: 'TsTypeAnnotation',
      span: [Object],
      typeAnnotation: [Object]
    }
  }
}

I'm working in Typescript, which might be the issue? At any rate, param.pat.type wouldn't match. It would be great to hear your thoughts on how best to proceed. I'll push where I'm up to to that branch.

from redwood.

Hi @Josh-Walker-GM , just an FYI I'm still working on this, very close now, so hoping to have a finished PR by tonight.

from redwood.

Hi @Josh-Walker-GM , #9639 is now ready for review. Hopefully all looks good!

from redwood.

Thanks @raph90! Again apologies about my sluggish responses, the PR is hugely appreciated! I'll review this as soon as I can.

from redwood.

Hi @Josh-Walker-GM and @Tobbe, I'm trying to run
yarn rw exp studio and I'm getting

Error: Cannot find module 'api/lib/config'

This is while running the project sync with my local redwood fork. Do you have any idea what might be causing this issue?

from redwood.

@raph90 Do you get any more info than just that error message? Any line numbers, a stack trace, or anything like that, that might help narrow the error down?

Also some more clear instructions on how to reproduce or exactly what you're doing to see that error could help us help you 🙂

from redwood.

@raph90 my gut reaction is that perhaps using a relative path might fix this but I haven't had too much time to think about this

from redwood.

Hi @Josh-Walker-GM and @Tobbe, yeah sorry for the very unspecific issue. I think there must have been another code change that affected our paths - I'm new to open source so just getting used to things 😆

For the paths, I've updated to a relative path. I tried to use the paths key of the tsconfig but for some reason I couldn't get that working.

I've updated this PR now with some new code - @Josh-Walker-GM I've removed the variables as you suggested, and I've now made it clear what the default exports are and what function / variable exports look like.

I've also needed to update the yarnrc.yml to get things working - I'm not sure if we want to add this as a redwood wide thing or if it should be restricted to the studio package, or perhaps if we just want to tell users they need to add it themselves.

But at any rate, this is now working, so hopefully you like what you see. Thanks!

from redwood.

That's weird about the yarnrc.yml updates. Unless @Josh-Walker-GM knows what's going on there we might have to bring in our yarn expert @jtoar to take a look

from redwood.

Closed in #9639

from redwood.