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

[10.x] add Storage::json() method to read and decode a json file #46548

Merged
merged 2 commits into from
Mar 22, 2023
Merged

[10.x] add Storage::json() method to read and decode a json file #46548

merged 2 commits into from
Mar 22, 2023

Conversation

lorenzolosa
Copy link
Contributor

Laravel v10.4.0 introduced a File::json() method to retrieve json encoded data from a file. That's nice, but often files are accessed using Storage and not File. Here I'm adding a similar Storage::json(), i.e.:

$data = Storage::json('sample.json');

which is equivalent to:

$contents = Storage::get('sample.json');
$data = json_decode($contents, true);

This is really the same as #46481, but for Storage instead of File.

@lorenzolosa lorenzolosa changed the title add Storage::json() method to read and decode a json file [10.x] add Storage::json() method to read and decode a json file Mar 22, 2023
@ankurk91
Copy link
Contributor

Can you add JSON_THROW_ON_ERROR option

@cosmastech
Copy link
Contributor

Can you add JSON_THROW_ON_ERROR option

Would it make sense to be able to pass all of the normal json_decode() args?

@taylorotwell taylorotwell merged commit 706a076 into laravel:10.x Mar 22, 2023
@taylorotwell
Copy link
Member

Flag support added to both methods.

* @param bool $lock
* @return array
*
* @throws \Illuminate\Contracts\Filesystem\FileNotFoundException
*/
public function json($path, $lock = false)
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this a breaking change? The $lock parameter is now third instead of second?

@driesvints
Copy link
Member

I've sent in a PR to revert this one.

@driesvints
Copy link
Member

It seems this isn't released yet so there's no breaking change here.

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.

6 participants