Upgrades `htmlparser2` to new major version `^8.0.0`.

apostrophecms / sanitize-html

Clean up user-submitted HTML, preserving whitelisted elements and whitelisted attributes on a per-element basis. Built on htmlparser2 for speed and tolerance

MIT License

3.68k stars 349 forks source link

Upgrades `htmlparser2` to new major version `^8.0.0`. #573

Closed kedarchandrayan closed 1 year ago

kedarchandrayan commented 1 year ago

Summary

Closes #565

What are the specific steps to test this change?

All existing behavior should remain same.

What kind of change does this PR introduce?

[ ] Bug fix
[ ] New feature
[x] Refactor
[ ] Documentation
[ ] Build-related changes
[x] Other - Upgrading htmlparser2 version

Make sure the PR fulfills these requirements:

[x] It includes a) the existing issue ID being resolved, b) a convincing reason for adding this feature, or c) a clear description of the bug it resolves
[x] The changelog is updated
[x] Related documentation has been updated
[x] Related tests have been updated

If adding a new feature without an already open issue, it's best to open a feature request issue first and wait for approval before working on it.

Other information:

kedarchandrayan commented 1 year ago

Hello @boutell, this is another PR as asked by you. It has only the change which was needed for the issue.

Test cases were changed to bring the behavior in sync with the new version of htmlparser2.

Please let me know your comments and changes needed.

Thanks, Kedar Chandrayan

boutell commented 1 year ago

Because of the parser option, which is passed on directly to htmlparser2, it is a backwards compatibility break to upgrade to a new major release of htmlparser2, unless the htmlparser2 API has not changed. Can you review the changelog of htmlparser2 to figure out if we need a major version bump for sanitize-html itself and/or a few bc wrappers to allow legacy htmlparser2 options to still work?

boutell commented 1 year ago

@kedarchandrayan reminder that there is an unanswered question blocking this PR:

Because of the parser option, which is passed on directly to htmlparser2, it is a backwards compatibility break to upgrade to a new major release of htmlparser2, unless the htmlparser2 API has not changed. Can you review the changelog of htmlparser2 to figure out if we need a major version bump for sanitize-html itself and/or a few bc wrappers to allow legacy htmlparser2 options to still work?

kedarchandrayan commented 1 year ago

Sure @boutell, I will go through the CHANGELOG of htmlparser2 for figuring out the need for a major update here. Sorry missed your last reply.

kedarchandrayan commented 1 year ago

Hello @boutell, htmlparser2 has src/Parser.ts which has interface ParserOptions for the parser options. The properties in this interface are the same in versions 6.0.0 and 8.0.1 (latest). So there is no change in the options.

Additionally, I went through the release changes mentioned here. Parser option change is not mentioned in any release.

Conclusion: We will not need a major bump for this PR.

boutell commented 1 year ago

Thanks!