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

Change html parser library to html-react-parser #9

Merged

Conversation

krzysztofwolski
Copy link
Contributor

@krzysztofwolski krzysztofwolski commented Sep 30, 2021

Why?

https://www.npmjs.com/package/react-html-parser seems to be not updated in a long time (4 years!)
https://www.npmjs.com/package/html-react-parser is under active maintenance and seems to be the drop-in replacement (which is confirmed by no changes in the snapshots :) )

My main motivation for this change was an investigation of my project bundle size. Change parser resulted in client bundle size from 800KB to 700KB for our project.

@krzysztofwolski
Copy link
Contributor Author

BTW Really wanted to say THANK YOU for your library! 🥇

@lightningspirit
Copy link
Contributor

Hi @krzysztofwolski thank you for your contribution 😄 Sorry for the delay in my response.

Yes, definitely makes sense to replace that package.

I just run the CI against your branch and it didn't pass because (I believe) it's using lockfileVersion@1 whereas your package-lock.json was generated with lockfileVersion@2.

This is the error message:

npm WARN read-shrinkwrap This version of npm is compatible with lockfileVersion@1, but package-lock.json was generated for lockfileVersion@2. I'll try to do my best with it!
...
Error: node_modules/html-react-parser/index.d.ts(3,31): error TS2307: Cannot find module 'htmlparser2' or its corresponding type declarations.

Anyway, we just updated to Node 14.x since 12.x has now entered half of his maintenance lifespan.

If you wan I can resolve the conflicting files for you, if you don't mind or you can correct them yourself.

Waiting for your response, thanks!

@krzysztofwolski
Copy link
Contributor Author

krzysztofwolski commented Oct 22, 2021

Sorry for the delay in my response.

No problem! :)

I've rebased with the master and used node14 + the latest version of the npm. hopefully, it will resolve the issue

@lightningspirit
Copy link
Contributor

Awesome! 👏 it worked. Thank you for your time spent and contribution. 🥇
Going to merge!

@lightningspirit lightningspirit merged commit 28f2ffb into moveyourdigital:master Oct 22, 2021
@krzysztofwolski krzysztofwolski deleted the change-html-parser-lib branch October 24, 2021 21:15
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