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

fix: add html unescape in hydrated data #250

Merged
merged 3 commits into from
Jul 16, 2024
Merged

Conversation

siesdart
Copy link
Contributor

Description

In Jaspr, data is escaped and sent to the client while hydrating. However, there is no unescape process in the client now, so data which contain escapable characters like '&' is different from the server's original on the client. I fixed this by adding unescape progress in the client.

Type of Change

  • 🛠️ Bug fix

Ready Checklist

  • I've read the Contribution Guide.
  • In case this PR changes one of the core packages, I've modified the respective CHANGELOG.md file using
    the semantic_changelog format.
  • I updated/added relevant documentation (doc comments with ///).
  • I added myself to the AUTHORS file (optional, if you want to).

@siesdart siesdart requested a review from schultek as a code owner July 13, 2024 13:49
Copy link

Package Version Report

The following packages have been updated:
jaspr : 0.13.3 -> 0.13.4
jaspr_builder : 0.13.3 -> 0.13.4
jaspr_cli : 0.13.3 -> 0.13.4
jaspr_tailwind : 0.2.0 -> 0.2.1
jaspr_test : 0.13.3 -> 0.13.4

@schultek
Copy link
Owner

I indeed missed the unescaping. However this can be a simple string replace operation since the only things that are escaped are

  • & to &
  • < to &lt;
  • > to &gt;

So something like str.replaceAll('&lt;', '<').replaceAll('&gt;', '>').replaceAll('&amp;', '&) or some more optimized version of that.

@siesdart
Copy link
Contributor Author

I modified the code as you said. Does it look better?

Copy link
Owner

@schultek schultek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the order so it doesn't affect itself. but then LGTM.

Thanks for this pr.

@schultek schultek merged commit 7909742 into schultek:main Jul 16, 2024
3 checks passed
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.

2 participants