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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Version 21.6.4 breaks interpolation #1721

Closed
mrlubos opened this issue Jan 18, 2022 · 14 comments
Closed

Version 21.6.4 breaks interpolation #1721

mrlubos opened this issue Jan 18, 2022 · 14 comments

Comments

@mrlubos
Copy link

@mrlubos mrlubos commented Jan 18, 2022

馃悰 Bug Report

Since version 21.6.4, translations with undefined variables print unescaped strings from the translations file. This behaviour was not observed in version 21.6.3 and hasn't been fixed since 21.6.4.

To Reproduce

See CodeSandbox. Change i18next version to 21.6.3 and observe the string {{ foo }} disappears which is the correct behaviour.

Expected behavior

The translation string is not printed, i.e. we print an empty string

Your Environment

  • i18next version: i.e. 21.6.4
  • os: Mac
@adrai
Copy link
Member

@adrai adrai commented Jan 19, 2022

Yes, this is expected, since skipOnVariables is true by default for v21.0.0 => https://www.i18next.com/misc/migration-guide#skiponvariables-true

If you still wish the old behaviour, you need to pass skipOnVariables: false like this: https://codesandbox.io/s/react-i18next-forked-cifxl?file=/src/i18n.js:345-367

@mrlubos
Copy link
Author

@mrlubos mrlubos commented Jan 19, 2022

Thanks @adrai! Although you can see this behaviour has been working as expected until 21.6.4, any prior 21.x.x release didn't break it

@adrai
Copy link
Member

@adrai adrai commented Jan 19, 2022

Yes, this was an error on previous versions

@adrai
Copy link
Member

@adrai adrai commented Jan 19, 2022

tldr; since v21.0.0 this was an error, and it was fixed in v21.6.4

@adrai adrai closed this as completed Jan 19, 2022
@bratzie
Copy link

@bratzie bratzie commented Jan 25, 2022

@adrai I recently ran into this issue as well. It feels to me like the intuitive behaviour is to hide the interpolation marker, i.e. {{interpolate}}, if you do not send in the value, or send in undefined explicitly. You can still get similar results if you explicitly send null as the variable, but not undefined or leaving it out completely.

I'm guessing the change was done due to security implications, but would there be any possibility to still hide the variables if they are not sent to the t function with skipOnVariables: true? Is there any documentation as to what security exploit this prevented? :o

@adrai
Copy link
Member

@adrai adrai commented Jan 25, 2022

Because of these:
=> #1490
#1479

{{ variable }} is kept, because it may be you want to have this in the translated text...

@adrai
Copy link
Member

@adrai adrai commented Jan 25, 2022

With v21.6.8 you can now pass in individual skipOnVariables option for the t function:

// resourceKey: "welcome": "{{ foo }}",

console.log(i18next.t('welcome', { foo: 'hi' })) // logs: "hi"
console.log(i18next.t('welcome', { foo: '' })) // logs: ""
console.log(i18next.t('welcome', { foo: null })) // logs: ""
console.log(i18next.t('welcome', { foo: undefined })) // logs: ""
console.log(i18next.t('welcome', {})) // logs: "{{ foo }}"
console.log(i18next.t('welcome', { interpolation: { skipOnVariables: false } })) // logs: ""

@bratzie
Copy link

@bratzie bratzie commented Jan 25, 2022

Those were interesting reads, and I definitely see the use of the option from reading #1479. I don't think that necessarily has to change your console log statement 4 and 5. In the latest version, with your example, passing { foo: undefined } logs {{ foo }} which feels inconsistent (that null logs "" and undefined doesn't).

I also don't see why fixing the recursive behaviour with $t() parsing has to enable not passing a variable at all to display {{ foo }} as I would expect "". Those feel unrelated?

This is what I would expect with the defaults in the latest 21.X.X version. (see sandbox here to observe passing undefined not working as you describe).

// resourceKey: "welcome": "{{ foo }}", "nested": "smurf"

console.log(i18next.t('welcome', { foo: 'hi' })) // logs: "hi"
console.log(i18next.t('welcome', { foo: '' })) // logs: ""
console.log(i18next.t('welcome', { foo: null })) // logs: ""
console.log(i18next.t('welcome', { foo: undefined })) // logs: "" => currently displays "{{ foo }}" as of 21.6.7
console.log(i18next.t('welcome', {})) // logs: "" => currently displays "{{ foo }}" as of 21.6.7
console.log(i18next.t('welcome', { foo: '$t(nested)' })) // logs: "$t(nested)"
console.log(i18next.t('welcome', { interpolation: { skipOnVariables: false } })) // logs: ""
console.log(i18next.t('welcome', { foo: '$t(nested)', interpolation: { skipOnVariables: false }})) // logs: "smurf"

@adrai
Copy link
Member

@adrai adrai commented Jan 25, 2022

This is what I would expect with the defaults in the latest 21.X.X version. (see sandbox here to observe passing undefined not working as you describe).

You're not using v21.6.8 in the sandbox yet.
This works: https://codesandbox.io/s/react-i18next-forked-7yd1f

@bratzie
Copy link

@bratzie bratzie commented Jan 25, 2022

Yeah, good point! Then the only thing that should still be odd (to me) would be the 5'th log where I would expect this behavior to make it work like it used to with skipOnVariables: false and pre version 21.

console.log(i18next.t('welcome', {})) // logs: "{{ foo }}" as of 21.6.8 => my expectation: ""
console.log(i18next.t('welcome')) // logs: "{{ foo }}" as of 21.6.8 => my expectation: ""

Or am I going crazy? :D

@adrai
Copy link
Member

@adrai adrai commented Jan 25, 2022

No, this will not change, I'm sorry.

@bratzie
Copy link

@bratzie bratzie commented Jan 25, 2022

Ok! Fair enough, if it is intended behavior then I'll work around it.

The fix with undefined really helped though! Thank you for that quick fix :)

@bratzie
Copy link

@bratzie bratzie commented Jan 25, 2022

Might still be a bug when passing skipOnVariables in the t function:

https://codesandbox.io/s/react-i18next-forked-j817j?file=/src/app.js

Passing skipOnVariables: false in init properly replaces with smurf.
Passing skipOnVariables: false in t function shows $t(nested) instead.

(I should be on latest as undefined behavior works as expected)

@adrai
Copy link
Member

@adrai adrai commented Jan 25, 2022

Yep, $t() usage was missing. v21.6.9 is better ;-)
https://codesandbox.io/s/react-i18next-forked-bhn6u?file=/src/index.js

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

No branches or pull requests

3 participants