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

[11.x] Switch Js::encode() to return HtmlString #49641

Closed
wants to merge 1 commit into from

Conversation

valorin
Copy link
Contributor

@valorin valorin commented Jan 10, 2024

Updating the Js::encode() helper to return an instance of HtmlString rather than a raw string.

The primary reason is to support using the normal escaping Blade tags: {{ Js::encode() }}, rather than {!! Js::encode() !}}, when using this in Blade. This will then match the behaviour already possible with Js::from() which returns an instance of Htmlable and supports {{ Js::from() }}.

Since this is a breaking change, it will need to go into v11 - hence the master branch.

There are no downsides to this change, it may just require minor code updates - which should be trivial to identify and perform with a search for Js::encode(.

@driesvints driesvints changed the title Switch Js::encode() to return HtmlString [11.x] Switch Js::encode() to return HtmlString Jan 11, 2024
@driesvints
Copy link
Member

Thanks. Paging in @calebporzio to see if this affects Livewire.

@taylorotwell taylorotwell marked this pull request as draft January 12, 2024 14:35
@driesvints driesvints changed the base branch from master to 11.x March 7, 2024 14:03
@driesvints driesvints marked this pull request as ready for review March 12, 2024 08:37
@driesvints
Copy link
Member

@calebporzio took a Quick Look at this one but didn't see anything wrong with it at first sight. So marking this as ready for review again.

@taylorotwell
Copy link
Member

Didn't quite get to this before L11 release.

@valorin
Copy link
Contributor Author

valorin commented Mar 12, 2024

@taylorotwell does that mean this'll need to wait for Laravel 12?

Can we apply this to master now so it doesn't get forgotten again?

@driesvints
Copy link
Member

Best to just attempt a PR @valorin

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

3 participants