-
Notifications
You must be signed in to change notification settings - Fork 16
Code Review Comments Typescript
When using the useTranslation() webhook, you don't need to include the common
translation file as it is included by default.
Prefer `const t = useTranslation('distribution');
Rather than
const t = useTranslation(['distribution','common']);
We're using react-i18next for localisations. Collections of translatable items are grouped into namespaces so that we can reduce bundle sizes and keep files contained to specific areas. The namespace files are json files - kept separate from the main bundles and downloaded on demand. These are also cached locally in the browser.
When using translations in your code, if you specify multiple name spaces, only first one will to be used by default.
const t = useTranslation(['distribution', 'app']);
t('button.add-item'); // This will use the distribution.json translation
t('button.open-the-menu', { ns: 'app' }); // This will use the app.json translation
the common
namespace is always included as a fallback option, so you do not need to specify it as a namespace
const t = useTranslation();
When translating strings, it's common to need a singular and plural option.
This can be accomplished using a _one
and _other
suffix on the translation key and passing a parameter to the translation request called 'count'.
Example
common.json
{
"permissions_one": "Permission",
"permissions_other": "Permissions"
}
example.ts
const t = useTranslation();
let permissionList = ['Permission a'];
let translation = t("permissions", {count: permissionList.length});
// translation will contain 'Permission'
let permissionList = ['Permission a', 'Permission b'];
let translation = t("permissions", {count: permissionList.length});
// translation will now contain 'Permissions'
Use of type assertion should be avoided, it can cause runtime errors in what looks like safe code. If type assertions have to be used, the resulting type should be consumed entirely in the same scope (with maximum amount of safety checks):
const myCanvas = document.getElementById("main_canvas") as HTMLCanvasElement;
if (!myCanvas) throw new Error("no canvas");
if (!myCanvas.getContext) throw new Error("not a canvas");
const gl = myCanvas.getContext("webgl", {
antialias: false,
depth: false,
});
Example of why type assertions can be dangerous
interface Check {
id: string,
willBeMissing: string
}
// ..
const doCheck = (check: Check) => {
console.log(check.id)
// ..
helper(check)
}
const helper = (check: Check) => {
console.log(check.willBeMissing.length) // runtime error (if check not Check)
}
// ..
const getLooksLikeCheck = () => ({ id: 'one' });
doCheck(getLooksLikeCheck() as Check) // runtime error
Union alternative to the above (with minimum changes)