conedevelopment / i18n

Push your Laravel translations to the front-end and use them easily with JavaScript.
https://pineco.de/using-laravels-localization-js/
MIT License
129 stars 21 forks source link

Update I18nServiceProvider.php #21

Closed RamyTalal closed 4 years ago

RamyTalal commented 4 years ago

Revert, this broke #12.

iamgergo commented 4 years ago

What is broken exactly?

RamyTalal commented 4 years ago

@iamgergo Only one package of a list of packages is translated. Basically the same bug in #12. When I reverted the code locally the issue was gone.

RamyTalal commented 4 years ago

@iamgergo Update: seems like a new issue. Unrelated to what I assumed.

afbeelding

iamgergo commented 4 years ago

Yes, it looks like your keys contain some special characters.

RamyTalal commented 4 years ago

It worked prior of the change. \u0000*\u0000items looks like the Laravel Collection is being encoded, I have seen this before.

iamgergo commented 4 years ago

I've never seen this. The \u0000 is null character: https://www.fileformat.info/info/unicode/char/0000/index.htm

RamyTalal commented 4 years ago

I know, this happens when the items property is being encoded. Adding })->toArray(); on line 98 fixes the issue.

iamgergo commented 4 years ago

And what if you add the to array at the very end? Just before printing the translations?

RamyTalal commented 4 years ago

On line 101? I get Call to a member function keys() on array

iamgergo commented 4 years ago

Okay, thanks! I'll soon make the release if nothing comes in.

RamyTalal commented 4 years ago

Thank you!

iamgergo commented 4 years ago

As it turns out, this change breaks the code and the tests as well.

Call to a member function merge() on array at /.../I18nServiceProvider.php:73)

I reverted this change, so we need an other solution for that.

RamyTalal commented 4 years ago

What about the initial code? Does that give you errors?

iamgergo commented 4 years ago

Probably not, I'll take a look. If it works well, then we bring back the old code.

RamyTalal commented 4 years ago

Do I need to do a new PR?

iamgergo commented 4 years ago

Yes, please it would be a help. Thanks!

RamyTalal commented 4 years ago

See https://github.com/thepinecode/i18n/pull/22