-
Notifications
You must be signed in to change notification settings - Fork 99
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
feat: add function to register custom keysets [WIP] #1844
base: main
Are you sure you want to change the base?
Changes from all commits
b890a9a
4f1c8a3
ac127b2
d774dea
cbae4c5
173f77c
089647d
a01e20c
0ef48ac
904b250
098d649
9dab09b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,169 @@ | ||
import React from 'react'; | ||
|
||
import type {KeysetData} from '@gravity-ui/i18n'; | ||
|
||
import {render, screen} from '../../../test-utils/utils'; | ||
import {i18n} from '../../i18n'; | ||
import {Dialog} from '../Dialog'; | ||
import {Pagination} from '../Pagination'; | ||
|
||
import {Lang, configure} from './configure'; | ||
import {registerCustomKeysets} from './registerCustomKeysets'; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it ok, that I don't have |
||
test('should render components with custom keysets', () => { | ||
registerCustomKeysets( | ||
'rs', | ||
createTestKeyset({ | ||
Dialog: { | ||
close: 'Затвори дијалог', | ||
}, | ||
Pagination: { | ||
button_previous: 'Претходно', | ||
button_next: 'Следеће', | ||
button_first: 'Прво', | ||
label_select_size: 'Изаберите величину странице', | ||
'label_input-placeholder': 'Стр.', | ||
'label_page-of': 'из', | ||
}, | ||
}), | ||
); | ||
registerCustomKeysets( | ||
'it', | ||
createTestKeyset({ | ||
Dialog: { | ||
close: 'Chiudere il dialogo', | ||
}, | ||
Pagination: { | ||
button_previous: 'Precedente', | ||
button_next: 'Avanti', | ||
button_first: 'Primo', | ||
label_select_size: 'Seleziona la dimensione della pagina', | ||
'label_input-placeholder': 'Pagina n.', | ||
'label_page-of': 'di', | ||
}, | ||
}), | ||
); | ||
|
||
configure({lang: 'rs'}); | ||
render(<TestComponents />); | ||
expect(screen.getByRole('button', {name: 'Затвори дијалог'})).toBeInTheDocument(); | ||
expect(screen.getByRole('button', {name: 'Претходно'})).toBeInTheDocument(); | ||
|
||
configure({lang: 'it'}); | ||
render(<TestComponents />); | ||
expect(screen.getByRole('button', {name: 'Chiudere il dialogo'})).toBeInTheDocument(); | ||
expect(screen.getByRole('button', {name: 'Precedente'})).toBeInTheDocument(); | ||
}); | ||
|
||
test('should render components with bundled keysets after a custom keysets registration', () => { | ||
registerCustomKeysets( | ||
'rs', | ||
createTestKeyset({ | ||
Dialog: { | ||
close: 'Затвори дијалог', | ||
}, | ||
Pagination: { | ||
button_previous: 'Претходно', | ||
button_next: 'Следеће', | ||
button_first: 'Прво', | ||
label_select_size: 'Изаберите величину странице', | ||
'label_input-placeholder': 'Стр.', | ||
'label_page-of': 'из', | ||
}, | ||
}), | ||
); | ||
|
||
configure({lang: Lang.En}); | ||
render(<TestComponents />); | ||
expect(screen.getByRole('button', {name: 'Close dialog'})).toBeInTheDocument(); | ||
expect(screen.getByRole('button', {name: 'Previous'})).toBeInTheDocument(); | ||
}); | ||
|
||
test('should override bundled keysets', () => { | ||
registerCustomKeysets( | ||
Lang.En, | ||
createTestKeyset({ | ||
Dialog: { | ||
close: '[Overriden] Close dialog', | ||
}, | ||
Pagination: { | ||
button_previous: '[Overriden] Previous', | ||
button_next: '[Overriden] Next', | ||
button_first: '[Overriden] First', | ||
label_select_size: '[Overriden] Select page size', | ||
'label_input-placeholder': '[Overriden] Page #', | ||
'label_page-of': '[Overriden] of', | ||
}, | ||
}), | ||
); | ||
|
||
configure({lang: Lang.En}); | ||
render(<TestComponents />); | ||
expect(screen.getByRole('button', {name: '[Overriden] Close dialog'})).toBeInTheDocument(); | ||
expect(screen.getByRole('button', {name: '[Overriden] Previous'})).toBeInTheDocument(); | ||
}); | ||
|
||
test('should throw an error if a component is not provided', () => { | ||
const keysetData = createTestKeyset({}); | ||
delete keysetData.Table; | ||
|
||
expect(() => registerCustomKeysets('rs', keysetData)).toThrow(); | ||
}); | ||
|
||
test('should throw an error if a component key is not provided', () => { | ||
const keysetData = createTestKeyset({}); | ||
Object.assign(keysetData, { | ||
Table: { | ||
label_empty: 'empty', | ||
// The values are omitted on purpose | ||
// 'label-actions': pluggedValue, | ||
// 'label-row-select': pluggedValue, | ||
}, | ||
}); | ||
|
||
expect(() => registerCustomKeysets('rs', keysetData)).toThrow(); | ||
}); | ||
|
||
test('should throw an error if excess components are provided', () => { | ||
const keysetData = createTestKeyset({ | ||
NonexistentComponent1: { | ||
label_cancel: 'cancel', | ||
label_submit: 'submit', | ||
}, | ||
NonexistentComponent2: { | ||
label_load: 'load', | ||
label_preload: 'preload', | ||
}, | ||
}); | ||
expect(() => registerCustomKeysets('it', keysetData)).toThrow(); | ||
}); | ||
|
||
test('should throw an error if excess component keys are provided', () => { | ||
const keysetData = createTestKeyset({ | ||
Alert: { | ||
label_close: 'cancel', | ||
nonexistent_key: 'nonexistent', | ||
nonexistent_key2: 'nonexistent2', | ||
}, | ||
}); | ||
expect(() => registerCustomKeysets('it', keysetData)).toThrow(); | ||
}); | ||
|
||
function TestComponents(): React.ReactElement { | ||
return ( | ||
<React.Fragment> | ||
<Dialog onClose={() => {}} open={true}> | ||
<Dialog.Header /> | ||
</Dialog> | ||
<Pagination page={1} pageSize={1} onUpdate={() => {}} /> | ||
</React.Fragment> | ||
); | ||
} | ||
|
||
// Custom keyset registration needs keysets for every component, or validation will fail. | ||
// We don't want to provide every keyset in every test, otherwise tests will be too verbose. | ||
// So we copy all the keysets from English, and then override the ones we'll test on. | ||
function createTestKeyset(dataToOverride: KeysetData): KeysetData { | ||
const keysetPrototype = JSON.parse(JSON.stringify(i18n.data.en)); | ||
return Object.assign(keysetPrototype, dataToOverride); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
import type {KeysetData} from '@gravity-ui/i18n'; | ||
|
||
import {i18n} from '../../i18n'; | ||
|
||
export const registerCustomKeysets = (language: string, data: KeysetData) => { | ||
validateCustomKeysets(language, data); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've decided to throw errors on startup, because this way developers will not have weird translation errors in their production, when we'll add new key to our component in a new major release for example. I think that their CI or dev environment manual tests will easily catch it. What do you guys, think? |
||
i18n.registerKeysets(language, data); | ||
}; | ||
|
||
function validateCustomKeysets(language: string, givenData: KeysetData): void { | ||
const trustedData = i18n.data.en; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We provide complete keysets in English language. So we can compare custom keysets with it. The code below looks kinda verbose, but IDK how to improve it, and it's very reliable and transparent about what needs to be fixed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also I think creating your first custom keyset might be daunting, because you'll only get a error one at a time. So maybe in our errors we should also show the templated English keyset. In the end our errors will look like this:
What do you think? |
||
|
||
const trustedComponents = Object.keys(trustedData); | ||
const givenComponents = Object.keys(givenData); | ||
|
||
trustedComponents.forEach((componentName) => { | ||
const trustedKeys = Object.keys(trustedData[componentName]); | ||
const givenKeys = Object.keys(givenData[componentName]); | ||
|
||
// Check that all component keys in trusted data exist in given data | ||
trustedKeys.forEach((keyName) => { | ||
if (givenData[componentName] === undefined) { | ||
throw createValidationError( | ||
language, | ||
`keyset for component '${componentName}' is required`, | ||
); | ||
} | ||
if (givenData[componentName][keyName] === undefined) { | ||
throw createValidationError( | ||
language, | ||
`key '${keyName}' for component '${componentName}' is required`, | ||
); | ||
} | ||
}); | ||
|
||
// Check for extra component keys | ||
if (trustedKeys.length !== givenKeys.length) { | ||
const keyDifference = getArrayDifference(trustedKeys, givenKeys); | ||
throw createValidationError( | ||
language, | ||
`excess component '${componentName}' keys for found ${JSON.stringify(keyDifference)}`, | ||
); | ||
} | ||
}); | ||
|
||
// Check for extra components | ||
if (trustedComponents.length !== givenComponents.length) { | ||
const componentDifference = getArrayDifference(trustedComponents, givenComponents); | ||
throw createValidationError( | ||
language, | ||
`excess components found ${JSON.stringify(componentDifference)}`, | ||
); | ||
} | ||
} | ||
|
||
function createValidationError(language: string, text: string): Error { | ||
return new Error(`Custom keysets '${language}' validation error: ${text}`); | ||
} | ||
|
||
function getArrayDifference(arr1: string[], arr2: string[]): string[] { | ||
const arr1Extra = arr1.filter((x) => !arr2.includes(x)); | ||
const arr2Extra = arr2.filter((x) => !arr1.includes(x)); | ||
|
||
return arr1Extra.concat(arr2Extra); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
// Use it for better autocomplete https://stackoverflow.com/a/61048124 | ||
export type AutocompleteSafeString = string & {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is concerning, but is needed for selecting custom keysets via
configure
. I've read all the codes, and I think it's alright. But what do you think, guys?