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

html-minifier causes React to throw reconciliation errors. #45

Open
cbilotta opened this issue Apr 6, 2017 · 6 comments
Open

html-minifier causes React to throw reconciliation errors. #45

cbilotta opened this issue Apr 6, 2017 · 6 comments
Labels

Comments

@cbilotta
Copy link
Member

cbilotta commented Apr 6, 2017

The HTML minifier is not aligned with React rendering because it does things such as :

  • Stripping whitespaces inside of elements, this is very good but if you don't manually strip the string yourself React will complain since it will render it with the whitespaces on the client.
  • Deleting the ; at the end on inline style-tags. Unfortunately, this is React's default behavior to put a ; at the end of inline style tags even if it's not necessary so it causes conflics.

I don't know about other differences between the react rendering and the html-minifier, but we should search for them and make sure it doesn't cause problems in the future.

I guess, the html-minifier should simply not do anything except removing whitespaces between tags.

@cbilotta cbilotta added the bug label Apr 6, 2017
@PEM--
Copy link
Member

PEM-- commented Apr 6, 2017

Indeed, concerning the inlined styles, this should be treated here: https://github.com/facebook/react/blob/master/src/renderers/dom/shared/CSSPropertyOperations.js#L172

@cbilotta
Copy link
Member Author

cbilotta commented Apr 6, 2017

Should I put an issue on React or html-minifier ? Not sure which one will listen 😄

@PEM--
Copy link
Member

PEM-- commented Apr 6, 2017

I'm doing it right now. I'll reference our issue in FB's repo.

@PEM--
Copy link
Member

PEM-- commented Apr 6, 2017

I've also referenced this issue on html-minifier as it should not alter inline styles when minifyCss is set to false.

@cbilotta
Copy link
Member Author

cbilotta commented Apr 6, 2017

I will make a commit where I comment the code responsible of making the minification happen for now. So we can keep on iterating the demo without having bugs.

@cbilotta
Copy link
Member Author

From my understanding, this has been fixed in both react and html-minifier.
I will do what is necessary to re-enable html minification in the develop branch and make sure it doesn't cause bugs anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants