[OP-19545] Replace lodash equality/clone helpers

`clone` becomes native shallow copies (spread / `[...]`). The 1:1
`isEqual`/`isEqualWith` deep comparisons switch from the global `_` to
tree-shakeable `lodash-es` imports — there is no safe native deep-equal.
`cloneDeep` likewise moves to `lodash-es` rather than `structuredClone`:
a filter's `$source` can hold `HalLink` instances whose `requestMethod`
function makes `structuredClone` throw `DataCloneError` on query save.
Three spec assertions move to vitest's `toEqual`.

Adds `lodash-es` + `@types/lodash-es` (same dependency the lodash-es
bucket introduces). The global `_` stays until the final cleanup.

https://community.openproject.org/wp/OP-19545
This commit is contained in:
Alexander Brandon Coles
2026-06-13 18:55:04 +01:00
parent 8d1cbe8000
commit 2cd907326b
23 changed files with 65 additions and 27 deletions
+18
View File
@@ -96,6 +96,7 @@
"json5": "^2.2.2",
"lit-html": "^3.3.3",
"lodash": "^4.18.1",
"lodash-es": "^4.17.21",
"luxon": "^3.7.2",
"mdx-embed": "^1.1.2",
"mime": "^4.1.0",
@@ -145,6 +146,7 @@
"@types/jquery": "^4.0.1",
"@types/jquery-migrate": "^3.3.3",
"@types/lodash": "^4.17.24",
"@types/lodash-es": "^4.17.12",
"@types/mousetrap": "^1.6.3",
"@types/node": "^25.9.1",
"@types/pako": "^2.0.4",
@@ -7031,6 +7033,16 @@
"dev": true,
"license": "MIT"
},
"node_modules/@types/lodash-es": {
"version": "4.17.12",
"resolved": "https://registry.npmjs.org/@types/lodash-es/-/lodash-es-4.17.12.tgz",
"integrity": "sha512-0NgftHUcV4v34VhXm8QBSftKVXtbkBG3ViCjs6+eJ5a6y6Mi/jiFGPc1sC7QK+9BFhWrURE3EOggmWaSxL9OzQ==",
"dev": true,
"license": "MIT",
"dependencies": {
"@types/lodash": "*"
}
},
"node_modules/@types/mousetrap": {
"version": "1.6.15",
"resolved": "https://registry.npmjs.org/@types/mousetrap/-/mousetrap-1.6.15.tgz",
@@ -12643,6 +12655,12 @@
"resolved": "https://registry.npmjs.org/lodash/-/lodash-4.18.1.tgz",
"integrity": "sha512-dMInicTPVE8d1e5otfwmmjlxkZoUpiVLwyeTdUsi/Caj/gfzzblBcCE5sRHV/AsjuCmxWrte2TNGSYuCeCq+0Q=="
},
"node_modules/lodash-es": {
"version": "4.18.1",
"resolved": "https://registry.npmjs.org/lodash-es/-/lodash-es-4.18.1.tgz",
"integrity": "sha512-J8xewKD/Gk22OZbhpOVSwcs60zhd95ESDwezOFuA3/099925PdHJ7OFHNTGtajL3AlZkykD32HykiMo+BIBI8A==",
"license": "MIT"
},
"node_modules/lodash.clonedeep": {
"version": "4.5.0",
"resolved": "https://registry.npmjs.org/lodash.clonedeep/-/lodash.clonedeep-4.5.0.tgz",
+2
View File
@@ -23,6 +23,7 @@
"@types/jquery": "^4.0.1",
"@types/jquery-migrate": "^3.3.3",
"@types/lodash": "^4.17.24",
"@types/lodash-es": "^4.17.12",
"@types/mousetrap": "^1.6.3",
"@types/node": "^25.9.1",
"@types/pako": "^2.0.4",
@@ -142,6 +143,7 @@
"json5": "^2.2.2",
"lit-html": "^3.3.3",
"lodash": "^4.18.1",
"lodash-es": "^4.17.21",
"luxon": "^3.7.2",
"mdx-embed": "^1.1.2",
"mime": "^4.1.0",
@@ -26,6 +26,7 @@
// See COPYRIGHT and LICENSE files for more details.
//++
import { isEqual } from 'lodash-es';
import { MultiInputState } from '@openproject/reactivestates';
import { WorkPackageResource } from 'core-app/features/hal/resources/work-package-resource';
import { Injectable, Injector } from '@angular/core';
@@ -88,7 +89,7 @@ export class WorkPackageCache extends StateCacheService<WorkPackageResource> {
// so that no consumer needs to call schema#$load manually
void this.schemaCacheService.ensureLoaded(wp).then(() => {
// Check if the work package has changed
if (skipOnIdentical && state.hasValue() && _.isEqual(state.value!.$source, wp.$source)) {
if (skipOnIdentical && state.hasValue() && isEqual(state.value!.$source, wp.$source)) {
debugLog('Skipping identical work package from updating');
return;
}
@@ -26,6 +26,7 @@
// See COPYRIGHT and LICENSE files for more details.
//++
import { isEqual } from 'lodash-es';
import { StateDeclaration, StateService, Transition, TransitionService, UIRouter } from '@uirouter/core';
import { IToast, ToastService } from 'core-app/shared/components/toaster/toast.service';
import { CurrentProjectService } from 'core-app/core/current-project/current-project.service';
@@ -129,7 +130,7 @@ export function uiRouterConfiguration(uiRouter:UIRouter, injector:Injector, modu
raw: true,
dynamic: true,
is: (val:unknown) => typeof (val) === 'string',
equals: (a:any, b:any) => _.isEqual(a, b),
equals: (a:unknown, b:unknown) => isEqual(a, b),
},
);
@@ -142,7 +143,7 @@ export function uiRouterConfiguration(uiRouter:UIRouter, injector:Injector, modu
raw: true,
dynamic: true,
is: (val:unknown) => typeof (val) === 'string',
equals: (a:unknown, b:unknown) => _.isEqual(a, b),
equals: (a:unknown, b:unknown) => isEqual(a, b),
},
);
}
@@ -228,7 +229,7 @@ export function initializeUiRouterListeners(injector:Injector) {
const hasProjectRoutes = toStateObject?.includes?.root;
const projectIdentifier = toParams.projectPath as string || currentProject.identifier;
if (hasProjectRoutes && !toParams.projects && projectIdentifier) {
const newParams = _.clone(toParams);
const newParams = { ...toParams };
_.assign(newParams, { projectPath: projectIdentifier, projects: 'projects' });
return $state.target(toState, newParams, { location: 'replace' });
}
@@ -102,7 +102,7 @@ export class HalLink implements HalLinkInterface {
throw new Error(`The link ${this.href} is not templated.`);
}
let href = _.clone(this.href) || '';
let href = this.href ?? '';
_.each(templateValues, (value:string, key:string) => {
const regexp = new RegExp(`{${key}}`);
href = href.replace(regexp, value);
@@ -34,6 +34,7 @@ import { LazyInject } from 'core-app/shared/helpers/angular/lazy-inject.decorato
import { HalLinkInterface } from 'core-app/features/hal/hal-link/hal-link';
import { ICKEditorContext } from 'core-app/shared/components/editor/components/ckeditor/ckeditor.types';
import idFromLink from 'core-app/features/hal/helpers/id-from-link';
import { cloneDeep } from 'lodash-es';
import isNewResource from 'core-app/features/hal/helpers/is-new-resource';
export type HalResourceClass<T extends HalResource = HalResource> = new(
@@ -175,7 +176,10 @@ export class HalResource {
}
public $plain():any {
return _.cloneDeep(this.$source);
// Use a deep clone (not structuredClone) because $source may contain
// HalResource instances (e.g. filter values), which carry functions and
// injector state that structuredClone cannot clone (DataCloneError).
return cloneDeep(this.$source);
}
public get $isHal():boolean {
@@ -26,6 +26,7 @@
// See COPYRIGHT and LICENSE files for more details.
//++
import { isEqual } from 'lodash-es';
import { ChangeDetectionStrategy, Component, EventEmitter, HostBinding, Input, OnInit, Output, inject } from '@angular/core';
import { I18nService } from 'core-app/core/i18n/i18n.service';
@@ -183,7 +184,7 @@ export class OpBaselineComponent extends UntilDestroyedMixin implements OnInit {
this.wpTableBaseline
.pristine$()
.subscribe((timestamps) => {
if (_.isEqual(timestamps, [DEFAULT_TIMESTAMP])) {
if (isEqual(timestamps, [DEFAULT_TIMESTAMP])) {
this.resetSelection();
this.wpTableBaseline.disable();
}
@@ -1,3 +1,4 @@
import { isEqualWith } from 'lodash-es';
import { Injector } from '@angular/core';
import { HalResource } from 'core-app/features/hal/resources/hal-resource';
import { WorkPackageResource } from 'core-app/features/hal/resources/work-package-resource';
@@ -110,7 +111,7 @@ export class GroupedRenderPass extends PlainRenderPass {
const joinedOrderedHrefs = (objects:any[]) => _.map(objects, (object) => object.href).sort().join(', ');
return _.isEqualWith(
return isEqualWith(
property,
group.href,
(a, b) => joinedOrderedHrefs(a) === joinedOrderedHrefs(b),
@@ -21,7 +21,7 @@ export class ChildRelationsRenderPass extends RelationsRenderPass {
}
// Render for each original row, clone it since we're modifying the tablepass
const rendered = _.clone(this.tablePass.renderedOrder);
const rendered = [...this.tablePass.renderedOrder];
const missingChildIds:string[] = [];
rendered.forEach((row:RowRenderInfo) => {
@@ -57,7 +57,7 @@ export class RelationsRenderPass {
}
// Render for each original row, clone it since we're modifying the tablepass
const rendered = _.clone(this.tablePass.renderedOrder);
const rendered = [...this.tablePass.renderedOrder];
rendered.forEach((row:RowRenderInfo) => {
// We only care for rows that are natural work packages
if (!row.workPackage) {
@@ -172,7 +172,7 @@ describe('UrlParamsHelper', () => {
pageSize: 100,
};
expect(_.isEqual(decodedQueryParams, expected)).toBeTruthy();
expect(decodedQueryParams).toEqual(expected);
});
});
@@ -264,7 +264,7 @@ describe('UrlParamsHelper', () => {
timestamps: 'PT0S',
};
expect(_.isEqual(v3Params, expected)).toBeTruthy();
expect(v3Params).toEqual(expected);
});
it('decodes custom options filters', () => {
@@ -324,7 +324,7 @@ describe('UrlParamsHelper', () => {
timestamps: 'PT0S',
};
expect(_.isEqual(v3Params, expected)).toBeTruthy();
expect(v3Params).toEqual(expected);
});
});
});
@@ -26,6 +26,7 @@
// See COPYRIGHT and LICENSE files for more details.
//++
import { isEqual } from 'lodash-es';
import { ChangeDetectionStrategy, ChangeDetectorRef, Component, ElementRef, Injector, Input, OnInit, inject } from '@angular/core';
import { StateService } from '@uirouter/core';
import { BehaviorSubject, combineLatest } from 'rxjs';
@@ -172,7 +173,7 @@ export class WorkPackageSingleViewComponent extends UntilDestroyedMixin implemen
.pipe(
this.untilDestroyed(),
map((resource) => this.contextFrom(resource)),
distinctUntilChanged<ResourceContextChange>((a, b) => _.isEqual(a, b)),
distinctUntilChanged<ResourceContextChange>((a, b) => isEqual(a, b)),
map(() => this.halEditing.changeFor(this.workPackage)),
)
.subscribe((changeset:WorkPackageChangeset) => this.refresh(changeset));
@@ -87,7 +87,7 @@ export class WpTableConfigurationTimelinesTabComponent implements TabComponent,
// Current label models
const { labels } = this.wpTableTimeline;
this.labels = _.clone(labels);
this.labels = { ...labels };
this.availableLabels = Object.keys(this.labels);
// Available labels
@@ -26,6 +26,7 @@
// See COPYRIGHT and LICENSE files for more details.
//++
import { isEqual } from 'lodash-es';
import { Injectable, inject } from '@angular/core';
import { QueryResource } from 'core-app/features/hal/resources/query-resource';
import { States } from 'core-app/core/states/states.service';
@@ -148,7 +149,7 @@ export class WorkPackageViewBaselineService extends WorkPackageQueryStateService
}
public hasChanged(query:QueryResource) {
return !_.isEqual(query.timestamps, this.current);
return !isEqual(query.timestamps, this.current);
}
public applyToQuery(query:QueryResource):boolean {
@@ -26,6 +26,7 @@
// See COPYRIGHT and LICENSE files for more details.
//++
import { isEqual } from 'lodash-es';
import { QueryResource } from 'core-app/features/hal/resources/query-resource';
import { States } from 'core-app/core/states/states.service';
import { Injectable, inject } from '@angular/core';
@@ -51,7 +52,7 @@ export class WorkPackageViewColumnsService extends WorkPackageQueryStateService<
public isCurrentlyEqualTo(a:QueryColumn[]) {
const comparer = (columns:QueryColumn[]) => columns.map((c) => c.href);
return _.isEqual(
return isEqual(
comparer(a),
comparer(this.getColumns()),
);
@@ -26,6 +26,7 @@
// See COPYRIGHT and LICENSE files for more details.
//++
import { isEqual } from 'lodash-es';
import { Injectable, inject } from '@angular/core';
import { combine, input, InputState } from '@openproject/reactivestates';
import { States } from 'core-app/core/states/states.service';
@@ -211,7 +212,7 @@ export class WorkPackageViewFiltersService extends WorkPackageQueryStateService<
public hasChanged(query:QueryResource) {
const comparer = (filter:HalResource[]) => filter.map((el) => el.$source);
return !_.isEqual(
return !isEqual(
comparer(query.filters),
comparer(this.rawFilters),
);
@@ -26,6 +26,7 @@
// See COPYRIGHT and LICENSE files for more details.
//++
import { isEqual } from 'lodash-es';
import { QueryResource } from 'core-app/features/hal/resources/query-resource';
import { States } from 'core-app/core/states/states.service';
import { Injectable, inject } from '@angular/core';
@@ -45,7 +46,7 @@ export class WorkPackageViewGroupByService extends WorkPackageQueryStateService<
public hasChanged(query:QueryResource) {
const comparer = (groupBy:QueryColumn|HalResource|null|undefined) => (groupBy ? groupBy.href : null);
return !_.isEqual(
return !isEqual(
comparer(query.groupBy),
comparer(this.current),
);
@@ -1,3 +1,4 @@
import { isEqual } from 'lodash-es';
import { QueryResource } from 'core-app/features/hal/resources/query-resource';
import { Injectable, inject } from '@angular/core';
import { States } from 'core-app/core/states/states.service';
@@ -63,7 +64,7 @@ export class WorkPackageViewHighlightingService extends WorkPackageQueryStateSer
public hasChanged(query:QueryResource) {
return query.highlightingMode !== this.current.mode
|| !_.isEqual(query.highlightedAttributes, this.current.selectedAttributes);
|| !isEqual(query.highlightedAttributes, this.current.selectedAttributes);
}
public applyToQuery(query:QueryResource):boolean {
@@ -26,6 +26,7 @@
// See COPYRIGHT and LICENSE files for more details.
//++
import { isEqual } from 'lodash-es';
import { combine } from '@openproject/reactivestates';
import { mapTo } from 'rxjs/operators';
import { Injectable, inject } from '@angular/core';
@@ -57,7 +58,7 @@ export class WorkPackageViewSortByService extends WorkPackageQueryStateService<Q
public hasChanged(query:QueryResource) {
const comparer = (sortBy:QuerySortByResource[]) => sortBy.map((el) => el.href);
return !_.isEqual(
return !isEqual(
comparer(query.sortBy),
comparer(this.current),
);
@@ -26,6 +26,7 @@
// See COPYRIGHT and LICENSE files for more details.
//++
import { isEqual } from 'lodash-es';
import { Injectable } from '@angular/core';
import { WorkPackageResource } from 'core-app/features/hal/resources/work-package-resource';
import { input } from '@openproject/reactivestates';
@@ -59,7 +60,7 @@ export class WorkPackageViewTimelineService extends WorkPackageQueryStateService
public hasChanged(query:QueryResource) {
const visibilityChanged = this.isVisible !== query.timelineVisible;
const zoomLevelChanged = this.zoomLevel !== query.timelineZoomLevel;
const labelsChanged = !_.isEqual(this.current.labels, query.timelineLabels);
const labelsChanged = !isEqual(this.current.labels, query.timelineLabels);
return visibilityChanged || zoomLevelChanged || labelsChanged;
}
@@ -26,6 +26,7 @@
// See COPYRIGHT and LICENSE files for more details.
//++
import { isEqual } from 'lodash-es';
import { AfterViewInit, ChangeDetectionStrategy, ChangeDetectorRef, Component, ElementRef, Injector, Input, ViewChild, inject } from '@angular/core';
import { I18nService } from 'core-app/core/i18n/i18n.service';
import { TimezoneService } from 'core-app/core/datetime/timezone.service';
@@ -163,7 +164,7 @@ export class OpWpDatePickerInstanceComponent extends UntilDestroyedMixin impleme
private isDifferentFromDatePickerSelectedDates(isoDates:string[]):boolean {
const datePickerSelectedDates = this.datePickerInstance.datepickerInstance.selectedDates;
const isoDatePickerSelectedDates = datePickerSelectedDates.map((date) => this.timezoneService.formattedISODate(date));
return !_.isEqual(isoDates, isoDatePickerSelectedDates);
return !isEqual(isoDates, isoDatePickerSelectedDates);
}
// set dates on flatpickr, trying to avoid jumping to a different month when possible
@@ -296,7 +296,7 @@ export class CkeditorAugmentedTextareaComponent extends UntilDestroyedMixin impl
private setupAttachmentRemovalSignal(editor:ICKEditorInstance) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-unsafe-member-access
this.attachments = _.clone((this.halResource as HalResource).attachments.elements);
this.attachments = [...(this.halResource as HalResource).attachments.elements];
this
.states
@@ -320,7 +320,7 @@ export class CkeditorAugmentedTextareaComponent extends UntilDestroyedMixin impl
editor.model.fire('op:attachment-removed', removedUrls);
}
this.attachments = _.clone(resource.attachments.elements);
this.attachments = [...resource.attachments.elements];
});
}
@@ -31,6 +31,7 @@ import {
InputState,
} from '@openproject/reactivestates';
import { take } from 'rxjs/operators';
import { cloneDeep } from 'lodash-es';
import { SchemaResource } from 'core-app/features/hal/resources/schema-resource';
import { FormResource } from 'core-app/features/hal/resources/form-resource';
@@ -425,9 +426,9 @@ export class ResourceChangeset<T extends HalResource = HalResource> {
// to let all default values be transmitted (type, status, etc.)
// We clone the object to avoid later manipulations to affect the original resource.
if (this.form$.value) {
payload = _.cloneDeep(this.form$.value.payload.$source);
payload = cloneDeep((this.form$.value.payload as HalResource).$source) as typeof payload;
} else {
payload = _.cloneDeep(this.pristineResource.$source);
payload = cloneDeep(this.pristineResource.$source) as typeof payload;
}
// Add attachments to be assigned.