Skip to content

Commit

Permalink
Avoid native private members #10451
Browse files Browse the repository at this point in the history
Because we still support Safari < 16.4, that means that native private
members are downleveled in a way that is less performant at runtime and
slightly bigger at bundle size.

See https://riegler.fr/blog/2024-05-17-private-fields-downleveling
  • Loading branch information
PowerKiKi committed May 22, 2024
1 parent 8886074 commit 71a45c8
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 51 deletions.
22 changes: 11 additions & 11 deletions projects/natural/src/lib/classes/abstract-detail.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,15 +91,15 @@ export class NaturalAbstractDetail<
*/
protected readonly route = inject(ActivatedRoute);

#dialogData: unknown = inject(MAT_DIALOG_DATA, {optional: true});
private _dialogData: unknown = inject(MAT_DIALOG_DATA, {optional: true});

/**
* Once set, this must not change anymore, especially not right after the creation mutation,
* so the form does not switch from creation mode to update mode without an actual reload of
* model from DB (by navigating to update page).
*/
#isUpdatePage = false;
readonly #changes = new CumulativeChanges<typeof this.form.getRawValue>();
private _isUpdatePage = false;
private readonly changes = new CumulativeChanges<typeof this.form.getRawValue>();

public constructor(
protected readonly key: string,
Expand All @@ -115,8 +115,8 @@ export class NaturalAbstractDetail<
if (this.isPanel) {
this.initForm();
} else {
const route = isNaturalDialogTriggerProvidedData(this.#dialogData)
? this.#dialogData.activatedRoute
const route = isNaturalDialogTriggerProvidedData(this._dialogData)
? this._dialogData.activatedRoute
: this.route;
this.#subscribeToModelFromResolvedData(route);
}
Expand Down Expand Up @@ -163,7 +163,7 @@ export class NaturalAbstractDetail<
* This should be used instead of checking `data.model.id` directly, in order to type guard and get proper typing
*/
protected isUpdatePage(): this is {data: {model: ExtractTone<TService>}} {
return this.#isUpdatePage;
return this._isUpdatePage;
}

/**
Expand All @@ -182,17 +182,17 @@ export class NaturalAbstractDetail<
ifValid(this.form).subscribe(() => {
const newValues = this.form.getRawValue();
if (submitAllFields) {
this.#changes.initialize({});
this.changes.initialize({});
}

const toSubmit = {
id: this.data.model.id,
...this.#changes.differences(newValues),
...this.changes.differences(newValues),
};

const update = now ? this.service.updateNow(toSubmit) : this.service.update(toSubmit);
update.subscribe(model => {
this.#changes.commit(newValues);
this.changes.commit(newValues);
this.alertService.info($localize`Mis à jour`);
this.postUpdate(model);
});
Expand Down Expand Up @@ -307,8 +307,8 @@ export class NaturalAbstractDetail<
* will incorrectly be called exactly 1 time per component instance, even if the object changes via route navigation.
*/
protected initForm(): void {
this.#isUpdatePage = !!this.data.model.id;
this._isUpdatePage = !!this.data.model.id;
this.form = this.service.getFormGroup(this.data.model);
this.#changes.initialize(this.form.getRawValue());
this.changes.initialize(this.form.getRawValue());
}
}
24 changes: 12 additions & 12 deletions projects/natural/src/lib/classes/cumulative-changes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@ import {ReadonlyDeep} from 'type-fest';
* Cumulate all changes made to an object over time
*/
export class CumulativeChanges<T extends Literal> {
#original: T = {} as T;
#diff: Partial<T> = {};
private original: T = {} as T;
private diff: Partial<T> = {};

/**
* Initialize the original values, should be called exactly one time per instance
*/
public initialize(originalValues: Readonly<T>): void {
this.#original = cloneDeep(originalValues);
this.#diff = {};
this.original = cloneDeep(originalValues);
this.diff = {};
}

/**
Expand All @@ -30,29 +30,29 @@ export class CumulativeChanges<T extends Literal> {
public differences(newValues: ReadonlyDeep<T>): Partial<T> | null {
Object.keys(newValues).forEach(key => {
if (
key in this.#diff ||
key in this.diff ||
(newValues[key] !== undefined &&
(!(key in this.#original) || !isEqual(this.#original[key], newValues[key])))
(!(key in this.original) || !isEqual(this.original[key], newValues[key])))
) {
(this.#diff as any)[key] = newValues[key];
(this.diff as any)[key] = newValues[key];
}
});

return Object.keys(this.#diff).length ? this.#diff : null;
return Object.keys(this.diff).length ? this.diff : null;
}

/**
* Commit the given new values, so they are not treated as differences anymore.
*/
public commit(newValues: ReadonlyDeep<T>): void {
this.#original = {
...this.#original,
this.original = {
...this.original,
...cloneDeep(newValues),
};

Object.keys(newValues).forEach(key => {
if (key in this.#diff && isEqual(this.#diff[key], newValues[key])) {
delete this.#diff[key];
if (key in this.diff && isEqual(this.diff[key], newValues[key])) {
delete this.diff[key];
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export class NaturalLinkableTabDirective extends NaturalAbstractController imple
* If false, disables the persistent navigation
*/
@Input() public naturalLinkableTab: boolean | '' = true;
#isLoadingRouteConfig = false;
private isLoadingRouteConfig = false;

public constructor(
private readonly component: MatTabGroup,
Expand All @@ -42,9 +42,9 @@ export class NaturalLinkableTabDirective extends NaturalAbstractController imple

router.events.pipe(takeUntilDestroyed()).subscribe(event => {
if (event instanceof RouteConfigLoadStart) {
this.#isLoadingRouteConfig = true;
this.isLoadingRouteConfig = true;
} else if (event instanceof RouteConfigLoadEnd) {
this.#isLoadingRouteConfig = false;
this.isLoadingRouteConfig = false;
}
});
}
Expand All @@ -68,7 +68,7 @@ export class NaturalLinkableTabDirective extends NaturalAbstractController imple

// When mat-tab-groups selected tab change, update url
this.component.selectedTabChange.pipe(takeUntil(this.ngUnsubscribe)).subscribe(event => {
if (this.#isLoadingRouteConfig) {
if (this.isLoadingRouteConfig) {
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,17 +76,17 @@ export class NaturalRelationsComponent<
@ViewChild(NaturalSelectComponent) private select?: NaturalSelectComponent<TService>;
@ContentChild(TemplateRef) public itemTemplate?: TemplateRef<unknown>;

#service!: TService;
private _service!: TService;

public get service(): TService {
return this.#service;
return this._service;
}

@Input({required: true})
public set service(service: TService) {
this.#service = service;
this._service = service;
this.loading = true;
const items$ = this.#service.watchAll(this.variablesManager).pipe(
const items$ = this._service.watchAll(this.variablesManager).pipe(
takeUntil(this.ngUnsubscribe),
tap({
next: () => (this.loading = false),
Expand Down
16 changes: 8 additions & 8 deletions projects/natural/src/lib/modules/sidenav/sidenav.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export class NaturalSidenavService extends NaturalAbstractController {

private minimizedStorageKeyWithName: string | null = null;
private openedStorageKeyWithName: string | null = null;
#isMobileView = false;
private _isMobileView = false;

public constructor(
public readonly breakpointObserver: BreakpointObserver,
Expand Down Expand Up @@ -113,8 +113,8 @@ export class NaturalSidenavService extends NaturalAbstractController {
.observe([Breakpoints.XSmall, Breakpoints.Small])
.pipe(takeUntil(this.ngUnsubscribe))
.subscribe(r => {
this.#isMobileView = r.matches;
const isBig = !this.#isMobileView;
this._isMobileView = r.matches;
const isBig = !this._isMobileView;
this.mode = isBig ? this.modes[0] : this.modes[1];

if (oldIsBig === null || isBig !== oldIsBig) {
Expand Down Expand Up @@ -147,14 +147,14 @@ export class NaturalSidenavService extends NaturalAbstractController {
}

public isMobileView(): boolean {
return this.#isMobileView;
return this._isMobileView;
}

/**
* Close nav on mobile view after a click
*/
public navItemClicked(): void {
if (this.#isMobileView) {
if (this._isMobileView) {
this.close();
}
}
Expand Down Expand Up @@ -199,7 +199,7 @@ export class NaturalSidenavService extends NaturalAbstractController {
const value = this.sessionStorage.getItem(this.openedStorageKeyWithName);

if (value === null) {
return !this.#isMobileView;
return !this._isMobileView;
} else {
return value === 'true';
}
Expand All @@ -224,9 +224,9 @@ export class NaturalSidenavService extends NaturalAbstractController {
public setOpened(value: boolean): void {
this.opened = value;

if (this.opened && this.#isMobileView) {
if (this.opened && this._isMobileView) {
this.minimized = false;
} else if (!this.#isMobileView) {
} else if (!this._isMobileView) {
assert(this.openedStorageKeyWithName);
this.sessionStorage.setItem(this.openedStorageKeyWithName, this.opened ? 'true' : 'false');
}
Expand Down
24 changes: 12 additions & 12 deletions projects/natural/src/lib/services/abstract-model.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export abstract class NaturalAbstractModelService<
private readonly creatingCache = new Map<Vcreate['input'] | WithId<Vupdate['input']>, Observable<Tcreate>>();
protected readonly apollo = inject(Apollo);
protected readonly naturalDebounceService = inject(NaturalDebounceService);
readonly #plural: string;
private readonly plural: string;

/**
*
Expand All @@ -67,11 +67,11 @@ export abstract class NaturalAbstractModelService<
protected readonly updateMutation: DocumentNode | null,
protected readonly deleteMutation: DocumentNode | null,
plural: string | null = null,
protected readonly createName: string | null = null,
protected readonly updateName: string | null = null,
protected readonly deleteName: string | null = null,
private readonly createName: string | null = null,
private readonly updateName: string | null = null,
private readonly deleteName: string | null = null,
) {
this.#plural = plural ?? makePlural(this.name);
this.plural = plural ?? makePlural(this.name);
}

/**
Expand Down Expand Up @@ -181,7 +181,7 @@ export abstract class NaturalAbstractModelService<
* You must subscribe to start getting results (and fetch from network).
*/
public getOne(id: string): Observable<Tone> {
return this.#prepareOneQuery(id, 'cache-and-network').pipe(
return this.prepareOneQuery(id, 'cache-and-network').pipe(
takeWhile(result => result.networkStatus !== NetworkStatus.ready, true),
map(result => (result.data as Literal)[this.name]),
);
Expand All @@ -198,10 +198,10 @@ export abstract class NaturalAbstractModelService<
* You **MUST** unsubscribe.
*/
public watchOne(id: string, fetchPolicy: WatchQueryFetchPolicy = 'cache-and-network'): Observable<Tone> {
return this.#prepareOneQuery(id, fetchPolicy).pipe(map(result => (result.data as Literal)[this.name]));
return this.prepareOneQuery(id, fetchPolicy).pipe(map(result => (result.data as Literal)[this.name]));
}

#prepareOneQuery(id: string, fetchPolicy: WatchQueryFetchPolicy): Observable<ApolloQueryResult<unknown>> {
private prepareOneQuery(id: string, fetchPolicy: WatchQueryFetchPolicy): Observable<ApolloQueryResult<unknown>> {
this.throwIfObservable(id);
this.throwIfNotQuery(this.oneQuery);

Expand Down Expand Up @@ -502,12 +502,12 @@ export abstract class NaturalAbstractModelService<
* This is used for the unique validator
*/
public count(queryVariablesManager: NaturalQueryVariablesManager<Vall>): Observable<number> {
const queryName = 'Count' + upperCaseFirstLetter(this.#plural);
const queryName = 'Count' + upperCaseFirstLetter(this.plural);
const filterType = upperCaseFirstLetter(this.name) + 'Filter';

const query = gql`
query ${queryName} ($filter: ${filterType}) {
count: ${this.#plural} (filter: $filter, pagination: {pageSize: 0, pageIndex: 0}) {
count: ${this.plural} (filter: $filter, pagination: {pageSize: 0, pageIndex: 0}) {
length
}
}`;
Expand Down Expand Up @@ -558,7 +558,7 @@ export abstract class NaturalAbstractModelService<
* This is used to extract only the array of fetched objects out of the entire fetched data
*/
protected mapAll(): OperatorFunction<FetchResult<unknown>, Tall> {
return map(result => (result.data as any)[this.#plural]); // See https://github.com/apollographql/apollo-client/issues/5662
return map(result => (result.data as any)[this.plural]); // See https://github.com/apollographql/apollo-client/issues/5662
}

/**
Expand All @@ -581,7 +581,7 @@ export abstract class NaturalAbstractModelService<
* This is used to extract only flag when deleting an object
*/
protected mapDelete(result: MutationResult<unknown>): Tdelete {
const name = this.deleteName ?? 'delete' + upperCaseFirstLetter(this.#plural);
const name = this.deleteName ?? 'delete' + upperCaseFirstLetter(this.plural);
return (result.data as any)[name]; // See https://github.com/apollographql/apollo-client/issues/5662
}

Expand Down

0 comments on commit 71a45c8

Please sign in to comment.