-
Notifications
You must be signed in to change notification settings - Fork 1k
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
[Bug?]: Redwood Studio can't load email templates (SWC) #9620
Comments
Assigning this one to you @Josh-Walker-GM, because mailer! 🙂 |
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:
However, there's then an issue with this line (216 in the compiled index.js from @redwoodjs/studio/dist/api/mail/index.js) If I make params optional:
and use them like this:
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! |
Thanks for providing such detail @raph90! Sorry I haven't gotten to this sooner. My initial reaction to your points are:
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! |
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.
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! |
@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 |
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. |
@Josh-Walker-GM here's a PR - let me know what you think, trying things my end and it does seem to work... EDIT
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. |
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
If I console log param there, it looks like this:
I'm working in Typescript, which might be the issue? At any rate, |
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. |
Hi @Josh-Walker-GM , #9639 is now ready for review. Hopefully all looks good! |
Thanks @raph90! Again apologies about my sluggish responses, the PR is hugely appreciated! I'll review this as soon as I can. |
Hi @Josh-Walker-GM and @Tobbe, I'm trying to run
This is while running the project sync with my local redwood fork. Do you have any idea what might be causing this issue? |
@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 🙂 |
@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 |
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 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! |
That's weird about the |
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. |
Reference to #9620. Currently Studio supports `export function Welcome()` syntax for emails. This PR adds support for `export const Welcome = () => ...` Also added optional dependencies for SWC to fix an error that _I think_ is architecture related. --------- Co-authored-by: Josh GM Walker <[email protected]>
Closed in #9639 |
…redwood into fix/enhance-error-apollo-suspense * 'fix/enhance-error-apollo-suspense' of github.com:dac09/redwood: (92 commits) chore(deps): update dependency @types/yargs to v17.0.32 (redwoodjs#9759) Make it easier to find useMatch docs (redwoodjs#9756) chore(unit tests): Use side-effect import to fix TS errors (redwoodjs#9754) fix(context): Refactor context (redwoodjs#9371) docs: Replaced deprecated <Set private> with PrivateSet within router.md (redwoodjs#9749) add TS support for storybook preview tsx config extension (redwoodjs#9309) fix(studio): Fix windows path issues (redwoodjs#9752) redwoodjs#9620: Update studio to support variable components (Mailer) (redwoodjs#9639) chore(tasks): Add comparison view to nmHoisting visualisation (redwoodjs#9751) chore(cli): make fs modules used in the CLI consistent (redwoodjs#9746) chore(tooling): Make sure console boxen print on a new line chore(CI): fix publish release candidate feat(CLI): add check node version middleware, rm `.nvmrc`, yarn engines (redwoodjs#9728) docs: added some clarification on serverless functions getting executed in a non-serverless environment (redwoodjs#9742) Fix sshExec() errors not displaying (redwoodjs#9743) chore(tooling): Add missing word in release tooling output Update Metadata docs (redwoodjs#9744) chore(CI): update test project fixture and CRWA for deploy target CI repo (redwoodjs#9730) chore(tooling): add script for getting nested dependency data (redwoodjs#9734) Trusted Documents docs: Proofreading corrections (redwoodjs#9737) ...
…ath-aliases * 'main' of github.com:redwoodjs/redwood: (92 commits) chore(deps): update dependency @types/yargs to v17.0.32 (redwoodjs#9759) Make it easier to find useMatch docs (redwoodjs#9756) chore(unit tests): Use side-effect import to fix TS errors (redwoodjs#9754) fix(context): Refactor context (redwoodjs#9371) docs: Replaced deprecated <Set private> with PrivateSet within router.md (redwoodjs#9749) add TS support for storybook preview tsx config extension (redwoodjs#9309) fix(studio): Fix windows path issues (redwoodjs#9752) redwoodjs#9620: Update studio to support variable components (Mailer) (redwoodjs#9639) chore(tasks): Add comparison view to nmHoisting visualisation (redwoodjs#9751) chore(cli): make fs modules used in the CLI consistent (redwoodjs#9746) chore(tooling): Make sure console boxen print on a new line chore(CI): fix publish release candidate feat(CLI): add check node version middleware, rm `.nvmrc`, yarn engines (redwoodjs#9728) docs: added some clarification on serverless functions getting executed in a non-serverless environment (redwoodjs#9742) Fix sshExec() errors not displaying (redwoodjs#9743) chore(tooling): Add missing word in release tooling output Update Metadata docs (redwoodjs#9744) chore(CI): update test project fixture and CRWA for deploy target CI repo (redwoodjs#9730) chore(tooling): add script for getting nested dependency data (redwoodjs#9734) Trusted Documents docs: Proofreading corrections (redwoodjs#9737) ...
Reference to #9620. Currently Studio supports `export function Welcome()` syntax for emails. This PR adds support for `export const Welcome = () => ...` Also added optional dependencies for SWC to fix an error that _I think_ is architecture related. --------- Co-authored-by: Josh GM Walker <[email protected]>
What's not working?
Due to some issue of filesystem access in the swc library, Redwood Studio can't dynamically load my React Email templates.
The error stack is this:
How do we reproduce the bug?
What's your environment? (If it applies)
Are you interested in working on this?
The text was updated successfully, but these errors were encountered: