Skip to content
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

better expose render methods #64

Closed
1 of 2 tasks
rolznz opened this issue Nov 18, 2022 · 5 comments
Closed
1 of 2 tasks

better expose render methods #64

rolznz opened this issue Nov 18, 2022 · 5 comments
Assignees
Labels
help wanted Extra attention is needed

Comments

@rolznz
Copy link

rolznz commented Nov 18, 2022

Unlike the example in the README, I have to import directly from the source:

import { render } from "@faire/mjml-react/dist/src/utils/render";

from maintainers

Ideally the goal would be:

import { render } from "@faire/mjml-react/renders";

Two blockers:

@emmclaughlin
Copy link
Collaborator

Thank you for pointing this out @rolznz! This change was made intentionally so that users who don't want to use this render import don't need to install its dependencies, but we hadn't updated to reflect this in the READEME. This PR should address it: #65

@emmclaughlin emmclaughlin self-assigned this Nov 18, 2022
@rolznz
Copy link
Author

rolznz commented Nov 18, 2022

@emmclaughlin I looked at the PR. Isn't there another way you can expose the import without having to import from the dist directory? this seems wrong to me.

@emmclaughlin
Copy link
Collaborator

We need to keep the render function out of the main index.ts file in order to fix dependency issues (e.g. #36). That said, agree it feels odd with the dist directory, will see if we can clean that up

faire-pr-bot-app bot pushed a commit that referenced this issue Nov 21, 2022
Lets update the readme to reduce confusion

related to #64
@IanEdington
Copy link
Contributor

If anyone has ideas of how to make this better for both esm and cjs we'd love to get help with this.

@IanEdington IanEdington added the help wanted Extra attention is needed label Nov 21, 2022
@IanEdington IanEdington changed the title render method is not exposed better expose render methods in both esm and cjs Nov 30, 2022
@IanEdington IanEdington changed the title better expose render methods in both esm and cjs better expose render methods Jan 9, 2023
@IanEdington
Copy link
Contributor

I think we can close this ticket and explore exporting to both esm and cjs in #72

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants