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

minify option is deprecated #58

Closed
Pedro-Souza opened this issue Apr 18, 2021 · 9 comments · Fixed by #75
Closed

minify option is deprecated #58

Pedro-Souza opened this issue Apr 18, 2021 · 9 comments · Fixed by #75

Comments

@Pedro-Souza
Copy link

The minify option is depreciated and when used a warning is triggered(see image). To solve it we could just remove the minify, what do you think?

PR that removed the minify option: mjmlio/mjml#2059

image

@botre
Copy link

botre commented May 25, 2021

You can toggle of the option explicitly:

const { html, errors } = render(email, {
  minify: false
});

Since this option is deprecated, I agree that the default should be false.

@andreymoser
Copy link

andreymoser commented Jul 30, 2021

I resolved using the same dependency (html-minifier) and minfy handling from mjml-core as the following:

minify(html, {
  collapseWhitespace: true,
  minifyCSS: false,
  caseSensitive: true,
  removeEmptyAttributes: true,
})

The minify option is really useful and I hope it helps, one good thing is to move this handling into mjml-react. :)

@daliusd
Copy link
Contributor

daliusd commented Feb 28, 2022

PRs are welcome

@pkuczynski
Copy link
Contributor

Here you go @daliusd #75. I couldn't run it locally as @wix/yoshi does not seem to exist anymore?

@daliusd
Copy link
Contributor

daliusd commented Mar 27, 2022

Yes, it does not work locally anymore.

@pkuczynski
Copy link
Contributor

Yes, it does not work locally anymore.

Shall this be fixed?

@daliusd
Copy link
Contributor

daliusd commented Mar 27, 2022

Yes, it does not work locally anymore.

Shall this be fixed?

It is complicated. We would need to drop @wix/yoshi to fix this.

@pkuczynski
Copy link
Contributor

I know its complicated, but maybe worth it, if this package is not public?

@daliusd
Copy link
Contributor

daliusd commented Mar 27, 2022

I know its complicated, but maybe worth it, if this package is not public?

It was public and is not public anymore. Most probably we will do something about it but we have other pressing problems to solve now and time will come to this problem as well eventually.

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 a pull request may close this issue.

5 participants