Skip to content

Commit

Permalink
perf(plugins): decrease number of calls to translate all extensions o…
Browse files Browse the repository at this point in the history 
…nly once (#1359)

* fix(common): translate extensions only once
found a couple of places in the code where it was translating the same things more than once
  • Loading branch information
@ghiscoding
ghiscoding committed Jan 23, 2024
1 parent 91426d1 commit 3e002f1
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 62 deletions.
11 changes: 8 additions & 3 deletions examples/vite-demo-vanilla-bundle/src/translate.service.ts
View file
Expand Up @@ -13,7 +13,7 @@ interface TranslateOptions {
export class TranslateService implements TranslaterService {
eventName = 'onLanguageChange' as TranslateServiceEventName;
protected _currentLanguage = 'en';
protected _locales: { [language: string]: Locales } = {};
protected _locales: { [language: string]: Locales; } = {};
protected _pubSubServices: PubSubService[] = [];
protected _options;
protected templateMatcher = /{{\s?([^{}\s]*)\s?}}/g;
Expand Down Expand Up @@ -82,17 +82,22 @@ export class TranslateService implements TranslaterService {
}

async use(newLang: string): Promise<Locales> {
const hasLangChanged = this._currentLanguage !== newLang;
this._currentLanguage = newLang;

// if it's already loaded in the cache, then resolve the locale set, else fetch it
if (this._locales?.hasOwnProperty(newLang)) {
this.publishLanguageChangeEvent(newLang);
if (hasLangChanged) {
this.publishLanguageChangeEvent(newLang);
}
return Promise.resolve(this._locales[newLang]);
}

const path = this._options.loadPath.replace(/{{lang}}/gi, newLang);
const localeSet = await this.fetchLocales(path, newLang);
this.publishLanguageChangeEvent(newLang);
if (hasLangChanged) {
this.publishLanguageChangeEvent(newLang);
}

return localeSet;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/extensions/extensionUtility.ts
View file
Expand Up @@ -19,7 +19,7 @@ export class ExtensionUtility {
* 3- else if nothing is provided use text defined as constants
*/
getPickerTitleOutputString(propName: string, pickerName: 'gridMenu' | 'columnPicker') {
if (this.sharedService.gridOptions && this.sharedService.gridOptions.enableTranslate && (!this.translaterService || !this.translaterService.translate)) {
if (this.sharedService.gridOptions?.enableTranslate && (!this.translaterService?.translate)) {
throw new Error('[Slickgrid-Universal] requires a Translate Service to be installed and configured when the grid option "enableTranslate" is enabled.');
}

Expand Down
24 changes: 7 additions & 17 deletions packages/common/src/services/__tests__/extension.service.spec.ts
View file
Expand Up @@ -28,15 +28,6 @@ import {
jest.mock('flatpickr', () => { });
const GRID_UID = 'slickgrid_12345';

const extensionUtilityStub = {
getPickerTitleOutputString: jest.fn(),
refreshBackendDataset: jest.fn(),
sortItems: jest.fn(),
translateItems: jest.fn(),
translateMenuItemsFromTitleKey: jest.fn(),
translateWhenEnabledAndServiceExist: jest.fn(),
} as unknown as ExtensionUtility;

const mockCellSelectionModel = {
pluginName: 'CellSelectionModel',
constructor: jest.fn(),
Expand Down Expand Up @@ -172,6 +163,7 @@ const extensionColumnPickerStub = {
};

describe('ExtensionService', () => {
let extensionUtility: ExtensionUtility;
let sharedService: SharedService;
let service: ExtensionService;
let translateService: TranslateServiceStub;
Expand All @@ -181,9 +173,10 @@ describe('ExtensionService', () => {
sharedService = new SharedService();
translateService = new TranslateServiceStub();
translateService.use('fr');
extensionUtility = new ExtensionUtility(sharedService, undefined, translateService);

service = new ExtensionService(
extensionUtilityStub,
extensionUtility,
filterServiceStub,
pubSubServiceStub,
sharedService,
Expand Down Expand Up @@ -351,7 +344,7 @@ describe('ExtensionService', () => {
const extSpy = jest.spyOn(SlickRowBasedEdit.prototype, 'init').mockImplementation();

service = new ExtensionService(
extensionUtilityStub,
extensionUtility,
filterServiceStub,
pubSubServiceStub,
sharedService,
Expand Down Expand Up @@ -672,7 +665,7 @@ describe('ExtensionService', () => {

it('should call the refreshBackendDataset method on the GridMenu Extension when service with same method name is called', () => {
const gridOptionsMock = { enableGridMenu: true } as GridOption;
const extSpy = jest.spyOn(extensionUtilityStub, 'refreshBackendDataset');
const extSpy = jest.spyOn(extensionUtility, 'refreshBackendDataset');
jest.spyOn(SharedService.prototype, 'gridOptions', 'get').mockReturnValue(gridOptionsMock);

service.refreshBackendDataset();
Expand All @@ -686,19 +679,15 @@ describe('ExtensionService', () => {
const cellMenuSpy = jest.spyOn(service, 'translateCellMenu');
const contextMenuSpy = jest.spyOn(service, 'translateContextMenu');
const colHeaderSpy = jest.spyOn(service, 'translateColumnHeaders');
const colPickerSpy = jest.spyOn(service, 'translateColumnPicker');
const contextSpy = jest.spyOn(service, 'translateContextMenu');
const gridMenuSpy = jest.spyOn(service, 'translateGridMenu');
const headerMenuSpy = jest.spyOn(service, 'translateHeaderMenu');

service.translateAllExtensions();

expect(cellMenuSpy).toHaveBeenCalled();
expect(contextMenuSpy).toHaveBeenCalled();
expect(colHeaderSpy).toHaveBeenCalled();
expect(colPickerSpy).toHaveBeenCalled();
expect(contextSpy).toHaveBeenCalled();
expect(gridMenuSpy).toHaveBeenCalled();
expect(headerMenuSpy).toHaveBeenCalled();
});

Expand Down Expand Up @@ -921,8 +910,9 @@ describe('ExtensionService', () => {
describe('without Translate Service', () => {
beforeEach(() => {
translateService = undefined as any;
extensionUtility = new ExtensionUtility(sharedService, undefined, translateService);
service = new ExtensionService(
extensionUtilityStub,
extensionUtility,
filterServiceStub,
pubSubServiceStub,
sharedService,
Expand Down
44 changes: 20 additions & 24 deletions packages/common/src/services/extension.service.ts
View file
Expand Up @@ -376,52 +376,51 @@ export class ExtensionService {
}

/** Translate all possible Extensions at once */
translateAllExtensions() {
translateAllExtensions(lang?: string) {
this.translateCellMenu();
this.translateColumnHeaders();
this.translateColumnPicker();
this.translateContextMenu();
this.translateGridMenu();
this.translateHeaderMenu();
this.translateRowEditPlugin();

// translating column headers will also indirectly translate ColumnPicker & GridMenu since headers are updated
// also make this the last call since it will also indirectly call `grid.invalidate()` which we want to do at the end only
this.translateColumnHeaders(lang);
}

/** Translate the Cell Menu titles, we need to loop through all column definition to re-translate them */
translateCellMenu() {
this._cellMenuPlugin?.translateCellMenu?.();
this._cellMenuPlugin?.translateCellMenu();
}

/** Translate the Column Picker and it's last 2 checkboxes */
translateColumnPicker() {
if (this._columnPickerControl?.translateColumnPicker) {
this._columnPickerControl.translateColumnPicker();
}
this._columnPickerControl?.translateColumnPicker();
}

/** Translate the Context Menu titles, we need to loop through all column definition to re-translate them */
translateContextMenu() {
this._contextMenuPlugin?.translateContextMenu?.();
this._contextMenuPlugin?.translateContextMenu();
}

/**
* Translate the Header Menu titles, we need to loop through all column definition to re-translate them
*/
translateGridMenu() {
this._gridMenuControl?.translateGridMenu?.();
this._gridMenuControl?.translateGridMenu();
}

/**
* Translate the Header Menu titles, we need to loop through all column definition to re-translate them
*/
translateHeaderMenu() {
this._headerMenuPlugin?.translateHeaderMenu?.();
this._headerMenuPlugin?.translateHeaderMenu();
}

/**
* Translate the action column buttons of the Row Based Edit Plugin
*/
translateRowEditPlugin() {
this._rowBasedEdit?.translate?.();
this._rowBasedEdit?.translate();
}

/**
Expand All @@ -430,12 +429,12 @@ export class ExtensionService {
* @param locale to use
* @param new column definitions (optional)
*/
translateColumnHeaders(locale?: boolean | string, newColumnDefinitions?: Column[]) {
translateColumnHeaders(locale?: string, newColumnDefinitions?: Column[]) {
if (this.sharedService && this.gridOptions && this.gridOptions.enableTranslate && (!this.translaterService || !this.translaterService.translate)) {
throw new Error('[Slickgrid-Universal] requires a Translate Service to be installed and configured when the grid option "enableTranslate" is enabled.');
}

if (locale && this.translaterService?.use) {
if (locale && this.translaterService?.use && this.translaterService.getCurrentLanguage() !== locale) {
this.translaterService.use(locale as string);
}

Expand All @@ -444,12 +443,13 @@ export class ExtensionService {
columnDefinitions = this.sharedService.columnDefinitions;
}

// translate all column headers & header column group when defined
this.translateItems(columnDefinitions, 'nameKey', 'name');
this.translateItems(this.sharedService.allColumns, 'nameKey', 'name');
this.translateItems(this.sharedService.allColumns, 'columnGroupKey', 'columnGroup');

// re-render the column headers
// re-render the column headers which will indirectly re-translate ColumnPicker/GridMenu
this.renderColumnHeaders(columnDefinitions, Array.isArray(newColumnDefinitions));
this._gridMenuControl?.translateGridMenu?.();
}

/**
Expand All @@ -461,7 +461,7 @@ export class ExtensionService {
if (!collection) {
collection = this.sharedService.columnDefinitions;
}
if (Array.isArray(collection) && this.sharedService.slickGrid && this.sharedService.slickGrid.setColumns) {
if (Array.isArray(collection) && this.sharedService.slickGrid?.setColumns) {
if (collection.length > this.sharedService.allColumns.length || forceColumnDefinitionsOverwrite) {
this.sharedService.allColumns = collection;
}
Expand All @@ -471,12 +471,14 @@ export class ExtensionService {
// replace Column Picker columns with newer data which includes new translations
if (this.gridOptions.enableColumnPicker && this._columnPickerControl) {
this._columnPickerControl.columns = this.sharedService.allColumns;
this._columnPickerControl.translateColumnPicker();
}

// replace the Grid Menu columns array list
if (this.gridOptions.enableGridMenu && this._gridMenuControl) {
this._gridMenuControl.columns = this.sharedService.allColumns ?? [];
this._gridMenuControl.recreateGridMenu();
this._gridMenuControl.translateGridMenu();
}
}

Expand Down Expand Up @@ -511,12 +513,6 @@ export class ExtensionService {
throw new Error('[Slickgrid-Universal] requires a Translate Service to be installed and configured when the grid option "enableTranslate" is enabled.');
}

if (Array.isArray(items)) {
items.forEach(item => {
if (item[inputKey]) {
item[outputKey] = this.translaterService?.translate(item[inputKey]);
}
});
}
this.extensionUtility.translateItems(items, inputKey, outputKey);
}
}
20 changes: 3 additions & 17 deletions packages/vanilla-bundle/src/components/slick-vanilla-grid-bundle.ts
View file
Expand Up @@ -737,17 +737,13 @@ export class SlickVanillaGridBundle<TData = any> {
// translate them all on first load, then on each language change
if (gridOptions.enableTranslate) {
this.extensionService.translateAllExtensions();
this.translateColumnHeaderTitleKeys();
this.translateColumnGroupKeys();
}

// on locale change, we have to manually translate the Headers, GridMenu
this.subscriptions.push(
this._eventPubSubService.subscribe('onLanguageChange', () => {
this._eventPubSubService.subscribe('onLanguageChange', (args: { language: string; }) => {
if (gridOptions.enableTranslate) {
this.extensionService.translateAllExtensions();
this.translateColumnHeaderTitleKeys();
this.translateColumnGroupKeys();
this.extensionService.translateAllExtensions(args.language);
if (gridOptions.createPreHeaderPanel && !gridOptions.enableDraggableGrouping) {
this.groupingService.translateGroupingAndColSpan();
}
Expand Down Expand Up @@ -1040,7 +1036,7 @@ export class SlickVanillaGridBundle<TData = any> {
}

if (this._gridOptions.enableTranslate) {
this.extensionService.translateColumnHeaders(false, newColumnDefinitions);
this.extensionService.translateColumnHeaders(undefined, newColumnDefinitions);
} else {
this.extensionService.renderColumnHeaders(newColumnDefinitions, true);
}
Expand Down Expand Up @@ -1447,16 +1443,6 @@ export class SlickVanillaGridBundle<TData = any> {
});
}

/** translate all columns (including hidden columns) */
protected translateColumnHeaderTitleKeys() {
this.extensionUtility.translateItems(this.sharedService.allColumns, 'nameKey', 'name');
}

/** translate all column groups (including hidden columns) */
protected translateColumnGroupKeys() {
this.extensionUtility.translateItems(this.sharedService.allColumns, 'columnGroupKey', 'columnGroup');
}

/**
* Update the "internalColumnEditor.collection" property.
* Since this is called after the async call resolves, the pointer will not be the same as the "column" argument passed.
Expand Down

0 comments on commit 3e002f1

Please sign in to comment.