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

In Trans.js, set default value to undefined as last possible option #462

Closed

Conversation

@dominicboutin
Copy link

dominicboutin commented Jun 28, 2018

Using Trans component, if node is left empty, default value is set to a empty string. I18next will use that default value instead of its fallback mechanism. By setting the default value to undefined will let i18next define what fallback to use.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jun 28, 2018

Coverage Status

Coverage remained the same at 81.557% when pulling 1152ab9 on dominicboutin:restore-fallback-mechanism into 844e7dd on i18next:master.

1 similar comment
@coveralls

This comment has been minimized.

Copy link

coveralls commented Jun 28, 2018

Coverage Status

Coverage remained the same at 81.557% when pulling 1152ab9 on dominicboutin:restore-fallback-mechanism into 844e7dd on i18next:master.

@jamuhl

This comment has been minimized.

Copy link
Member

jamuhl commented Jun 28, 2018

isn't defaultValue not just used in case of the fallback not finding something: https://github.com/i18next/i18next/blob/master/src/Translator.js#L122

could you describe more / or make a sample what fallback mechanism you mean?

@dominicboutin

This comment has been minimized.

Copy link
Author

dominicboutin commented Jun 28, 2018

Exactly, since defaultValue is set to a empty string and res is undefined,
https://github.com/i18next/i18next/blob/master/src/Translator.js#L126
usedkey isn't set to true
res isn't set to key

plus that skip the parseMissingKeyHandler function
https://github.com/i18next/i18next/blob/master/src/Translator.js#L177

@jamuhl

This comment has been minimized.

Copy link
Member

jamuhl commented Jun 28, 2018

cause empty string is considered valid value per default: https://github.com/i18next/i18next/blob/master/src/defaults.js#L31

potentially this could break existing projects. could we make it a pick from reactI18nextOptions.defaultTransDefaultValueFallback which defaults to empty string so it will be backwards compatible

@jamuhl

This comment has been minimized.

Copy link
Member

jamuhl commented Jun 28, 2018

beside how are you using the Trans component to not have a meaningful defaultValue...do you use it everywhere to just render plain strings not having a component interpolated? if so that could be done by just using the t function from the hoc, render prop

@dominicboutin

This comment has been minimized.

Copy link
Author

dominicboutin commented Jun 28, 2018

We are using TypeScript, its easier to use Trans in our components than set the prop type to include t.
<Trans i18nKey="somekey" />

Setting returnEmptyString to false fixes our issues but I feel like the default Trans value should undefined.
Adding a reactI18nextOptions.defaultTransDefaultValueFallback could be a solution or wait for the next major version.

On our side the issue is fixed with that workaround, this PR can be closed.

@jamuhl

This comment has been minimized.

Copy link
Member

jamuhl commented Jun 29, 2018

will keep this open for now - will add the defaultTransDefaultValueFallback asap. So you got that option.

@jamuhl

This comment has been minimized.

Copy link
Member

jamuhl commented Oct 25, 2018

closing this for now...added an issue to keep that for future development: #577

sorry for not yet having time to do so...a PR would be welcome

@jamuhl jamuhl closed this Oct 25, 2018
@jamuhl

This comment has been minimized.

Copy link
Member

jamuhl commented Dec 31, 2018

published in react-i18next@9.0.0

allow defining transEmptyNodeValue in i18next.options.react to set a default fallback value

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.