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

fix(mjml-core): proper validation of mjml tag #2709

Closed
wants to merge 1 commit into from

Conversation

returnv01d
Copy link

@returnv01d returnv01d commented Jul 13, 2023

Fixes #2695
So, the dependency rules were not working correctly because <mjml> tag was entirely ignored. Without it MJML wasn't working, so I made new component that represents the <mjml> tag , so dependency rules can work properly. It can encapsulate document, for example it can be used to generate skeleton code here in render instead of special behavior for that, but it needs more refactoring then.
Couldn't find place in package to write tests so here is proof of validity:
https://github.com/mjmlio/mjml/assets/32848266/39905889-2f15-4500-b405-4edd02e91110

@returnv01d returnv01d marked this pull request as ready for review July 13, 2023 20:35
Comment on lines +1 to +11
import { BodyComponent } from 'mjml-core'

export default class MjDocument extends BodyComponent {
static componentName = 'mjml'

static endingTag = false

render() {
return ''
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really happy with this solution here, body component expect to render something, it's just a hack at the end to make it work :/

I guess we could add something like a "mock-class" inside MJML just here to list mjml attributes + dependencies but it doesn't really need to have it's own package and should live inside mjml-core

@iRyusa
Copy link
Member

iRyusa commented Oct 2, 2023

Closing for now we won't go in this direction I guess.

@iRyusa iRyusa closed this Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rendering another MJML template in MJML template
2 participants