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

Design decision issue: useTranslation hook returning array vs object #712

Closed
kachkaev opened this issue Feb 6, 2019 · 10 comments
Closed

Comments

@kachkaev
Copy link
Contributor

kachkaev commented Feb 6, 2019

👋 I've just discovered v10 (the hooks one) and am pretty excited by it! Here's a question about one architectural decision that has come to my mind: Why is the result of useTranslation() an array rather than an object?

const [ t, i18n ] = useTranslation();
// vs.
const { t, i18n } = useTranslation();

Arrays are good for things that can have arbitrary names, e.g. [x, setX] = useState(), which is not the case for react-i18next's useTranslation(). As a developer, I'd like t to be t in all components, same for i18n. Doing something like const [ abc, def ] = useTranslation(); would mean the component's code is harder to understand by the developers, which a sign of not following a good practice. On the other hand, if the result of the hook is an object, I can do const { i18n } = useTranslation(), e.g. when I only need to know the current language in a component. No need to have [, i18n] in this case.

Alternatively, the hook could be like

const i18n = useI18n();

// when only t is needed
const { t } = useI18n();

// i18n + shortcut to t
const i18n = useI18n();
const t = i18n.t;

@jamuhl If I've missed a conversation about the hook design, could please point me to one? Otherwise, what are your thoughts on the current hook design? How did you justify it?

@jamuhl
Copy link
Member

jamuhl commented Feb 6, 2019

that architectural decision was made by react: https://reactjs.org/docs/hooks-intro.html

@kachkaev
Copy link
Contributor Author

kachkaev commented Feb 6, 2019

Hooks don't have to return an array – even built-in useContext(), useMemo(), useCallback() etc. do not do that.

You can check out https://usehooks.com/ with a few examples, some of which returning arrays, some – objects. E.g. for useHistory() one it makes sense to return a spreadable object rather than an array – otherwise there are too many items to cycle through before you get to the one you need. Looking at various hooks, I can say that there are more hooks that don't return an array than the ones that do. This is what makes me wonder what return typ a hook in react-i18next should ideally have. 🤔

@jamuhl
Copy link
Member

jamuhl commented Feb 6, 2019

useState and useReducer do -> so i guess it will be what users will expect

@kachkaev
Copy link
Contributor Author

kachkaev commented Feb 6, 2019

Although these two hooks are common, I expect a whole swarm of other popular hooks out there, which won't be using this return pattern for a good reason. So I don't think there will be an expectation issue if a hook from react-i18next returns something other than an array with two elements.

Besides, if you look closer at useState(), useReducer() or other array-returning hooks on https://usehooks.com/, you'll notice that the most common pattern there is [value, setValue] = useSomething(). That make sense to me. If getter / setter pattern stays popular, there can even rise a small confusion when someone sees const [ t, i18n ] = useTranslation();. Is the second argument a function that somehow changes the value in first one?

@jamuhl
Copy link
Member

jamuhl commented Feb 6, 2019

🤷‍♂️there was a full 4 months of existing hook version - honestly not the time to change the API now. It's decided and will stay like it is. Just to late -> v10 is out...people using the alpha of v10 for long.

Appreciate your feedback but it's set -> still you can create a own hook reusing our if you prefer another API.

@jamuhl jamuhl closed this as completed Feb 6, 2019
@kachkaev
Copy link
Contributor Author

kachkaev commented Feb 6, 2019

No worries. Just wanted to share thoughts. Sorry that I missed alphas – totally understand that this feedback would be much more relevant a few months back.

@kachkaev
Copy link
Contributor Author

kachkaev commented Feb 7, 2019

Closed by #714 🎉

@NeoLegends
Copy link

One point in favor of the array syntax is superior minification abilities. Object keys won't be minified in most cases, because the minifier probably isn't going to have enough information to ensure correctness during the transformation. Destructuring, however, doesn't suffer from this problem. Since it's the user giving out the names to the array elements, the minifier is free to rename them as it wishes.

@kachkaev
Copy link
Contributor Author

That's true. However given the already short names as t and i18n this optimisation would be insignificant. i18n would be picked up by gzip and compressed into something that's just over one byte per instance – not much more than occupied by a single letter or digit.

@timkrins
Copy link

TIL: you can add keyed values to a JS Array object... because, of course, it's an Object!

that's freaking cool!

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

4 participants