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

Bump mrml from 3.1.5 to 4.0.0 in /native/mjml_nif #152

Merged
merged 1 commit into from
Jul 10, 2024

Conversation

moomerman
Copy link
Contributor

Version 4.0.0 fixes a bug if you use phoenix function components
to generate MJML and have heex annotations enabled by default.

Essentially the heex annotations add comments outside the tag
and this was breaking the pareser. The bug has been fixed upstream.

See jdrouet/mrml#409 for more info.

@moomerman
Copy link
Contributor Author

moomerman commented Jun 18, 2024

I tried to update to the latest MJML release but the tests are giving the confusing error:

error: failed to select a version for `mrml`.
    ... required by package `mjml_nif v4.0.0 (/home/runner/work/mjml_nif/mjml_nif/native/mjml_nif)`
versions that meet the requirements `^4.0.0` are: 4.0.0

It looks like it is looking for 4.0.0 and found a 4.0.0 release but couldn't use it

[Edit: I spotted that it needed to be "4.0" not "4.0.0" 🤦🏼 ]

Version 4.0.0 fixes a bug if you use phoenix function components
to generate MJML and have heex annotations enabled by default.

Essentially the heex annotations add comments outside the <mjml> tag
and this was breaking the pareser.  The bug has been fixed upstream.

See jdrouet/mrml#409 for more info.
mrml = { version = "4.0", default-features = false, features = [
"parse",
"render",
] }
Copy link
Member

@paulgoetze paulgoetze Jul 8, 2024

Choose a reason for hiding this comment

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

Thanks @moomerman for the PR, very much appretiated 🙏.

What's the reason for removing the "oderedmap" feature? I saw that it's enabled by default since mrml 2.0, but since we don't use all default features (default-features = false) I think we need to keep it in order to make sure the order of the created HTML is consistent (see #63)?

Copy link
Contributor Author

@moomerman moomerman Jul 9, 2024

Choose a reason for hiding this comment

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

I don't fully understand it, but it fails to compile with orderedmap in:

cargo build
warning: `/Users/richard/workspace/moomerman/mjml_nif/native/mjml_nif/.cargo/config` is deprecated in favor of `config.toml`
note: if you need to support cargo 1.38 or earlier, you can symlink `config` to `config.toml`
error: failed to select a version for `mrml`.
    ... required by package `mjml_nif v4.0.0 (/Users/richard/workspace/moomerman/mjml_nif/native/mjml_nif)`
versions that meet the requirements `^4.0` (locked to 4.0.0) are: 4.0.0

the package `mjml_nif` depends on `mrml`, with features: `orderedmap` but `mrml` does not have these features.


failed to select a version for `mrml` which could resolve this conflict

It look like it was removed upstream.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, yes, you are right of course: mrml only uses indexmap (i.e. the former orderemap feature) from 4.0 on and I did not see it, when I first scrolled through the changelog: https://github.com/jdrouet/mrml/blob/87bec4bddcd3b9601d1f06b67e8b996512ed9e07/packages/mrml-core/CHANGELOG.md?plain=1#L38

So I'll merge your changes and prepare a new release later today. Thanks again! 💜

@paulgoetze paulgoetze merged commit 7244c52 into adoptoposs:main Jul 10, 2024
11 checks passed
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.

None yet

2 participants