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

handle optional closing tags for self-closing tags. fixes #41 #81

Merged
merged 11 commits into from
Oct 30, 2023
Merged

handle optional closing tags for self-closing tags. fixes #41 #81

merged 11 commits into from
Oct 30, 2023

Conversation

mreinstein
Copy link
Contributor

fixes:

This changes causes the tests from f1803d5 to pass, and they've been included too.

@mreinstein
Copy link
Contributor Author

Some follow up notes on this:

  • there is some noise in this PR (some extra unneeded commits.) I'm happy to send a clean PR if this looks like something we want
  • I made a few small style changes to the code, because it's just really dense and hard to read with everything so squashed together. If we're really against these changes I'm willing to re-submit without those formatting changes.
  • while these changes still pass all the existing unit tests, we would need to do a semver major bump if this were to get released.
  • if we're going to bother making breaking changes, are there other parser fixes we want to make?
  • does it make sense to drop cjs in favor of a pure esm module, since we're making a breaking api change anyway?

@mreinstein
Copy link
Contributor Author

@mreinstein
Copy link
Contributor Author

@goto-bus-stop despite being a maintainer, I can't seem to approve my own PRs. Would you be willing to review/approve this? I've been using these changes for a while.

@mreinstein mreinstein merged commit 16690b2 into choojs:master Oct 30, 2023
@mreinstein
Copy link
Contributor Author

nevermind, I'm not sure why I can merge from this page, but not from the PR view.

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.

1 participant