Skip to content

Clarify @json usage in element attributes #5060

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

Merged
merged 2 commits into from
Mar 16, 2019

Conversation

Travis-Britz
Copy link
Contributor

@Travis-Britz Travis-Britz commented Mar 14, 2019

If I tried using double quotes instead of single quotes around attr='@json', I'm sure somebody else will.

@browner12
Copy link
Contributor

I feel like this goes more towards correct Vue usage, rather than something specific to the Blade directive.

@Travis-Britz
Copy link
Contributor Author

Travis-Britz commented Mar 14, 2019

@browner12 I used the Vue component for the example because I thought Vue is the most common use case for data attributes in Laravel applications, and the example Vue component is already included in every new Laravel install. However, I specifically referenced data-* attributes for cases like <ul data-items='@json($array)'></ul> that would have nothing to do with Vue, and which are also common.

There are basically two ways to seed data on a page for javascript: set a variable, or place it in an element. I think including a correct example of the latter can only help.

@browner12
Copy link
Contributor

are you sure that single quotes are required?

my guess is it has to do with correctly escaping characters when output to HTML. Are you overriding the default encoding options of @json? I think it's important to have JSON_HEX_APOS and JSON_HEX_QUOT enabled when outputting to HTML.

http://php.net/manual/en/json.constants.php

@Travis-Britz
Copy link
Contributor Author

Travis-Britz commented Mar 14, 2019

@browner12 yes, single quotes are required because the json output uses double quotes.

For example, compare the output:

data-tasks='@json($tasks)'
data-tasks="@json($tasks)"
data-tasks='[{"id":70,"description":"foo"}]'
data-tasks="[{"id":70,"description":"foo"}]"

The second one will fail because you end up with [{ as the attribute value, and then random garbage after that (and probably most of the json displayed directly on the page).

And no, I'm not overriding the encoding options. The @json directive uses these options for json_encode by default: JSON_HEX_TAG | JSON_HEX_APOS | JSON_HEX_AMP | JSON_HEX_QUOT.
https://github.com/laravel/framework/blob/e7064ca184e395f86fc924e0d58d94955c734675/src/Illuminate/View/Compilers/Concerns/CompilesJson.php#L12

So quotes within the string are escaped with their \u escape sequence, but the quotes around the string itself are not escaped.

If you want double quotes on the attribute, then you have to use something like data-tasks="{{ json_encode() }}" which will turn quotes from the json into &quot;... but then you're not using the @json directive, which is what this PR is documenting in the first place.

@browner12
Copy link
Contributor

I guess that's why I'm confused. If you're using the default encoding, you won't run into the issue with the double quotes, correct?

@Travis-Britz
Copy link
Contributor Author

Travis-Britz commented Mar 14, 2019

The default encoding will encode the quotes inside the string.

So this description: "foo" 'bar'

will be encoded to

[{"id":72,"description":"\u0022foo\u0022 \u0027bar\u0027"}]

So the only way to include the json object (made up of escaped attributes and strings) inside an html attribute value is to surround it with single quotes:

<ul data-tasks='[{"id":72,"description":"\u0022foo bar\u0022"}]'></ul>

Because json object has literal double quotes, unescaped. Using double quotes would terminate the attribute value early.

@browner12
Copy link
Contributor

ahh, got it. thanks for clearing that up for me.

@Travis-Britz
Copy link
Contributor Author

No problem! I was confused too, hence the PR.

@taylorotwell taylorotwell merged commit 6c99b30 into laravel:5.8 Mar 16, 2019
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.

3 participants