Skip to content

feat: Normalize into immutablejs state #3506

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 31 additions & 11 deletions packages/normalizr/src/normalize/NormalizeDelegate.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {
import type {
EntityTable,
NormalizedIndex,
INormalizeDelegate,
Expand All @@ -7,12 +7,22 @@ import {
import { getCheckLoop } from './getCheckLoop.js';
import { POJODelegate } from '../delegate/Delegate.js';
import { INVALID } from '../denormalize/symbol.js';
import { NormalizedSchema } from '../types.js';

/** Full normalize() logic for POJO state */
export class NormalizeDelegate
extends POJODelegate
implements INormalizeDelegate
implements INormalizeDelegate, NormalizedSchema<EntityTable, any>
{
// declare readonly normalized: NormalizedSchema<E, R>;
Copy link
Preview

Copilot AI Aug 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commented declare statement should be removed as it's unused and adds confusion about the class structure.

Suggested change
// declare readonly normalized: NormalizedSchema<E, R>;

Copilot uses AI. Check for mistakes.

declare result: any;
declare readonly entities: EntityTable;
declare readonly indexes: {
[entityKey: string]: {
[indexName: string]: { [lookup: string]: string };
};
};

declare readonly entitiesMeta: {
[entityKey: string]: {
[pk: string]: {
Expand All @@ -26,8 +36,10 @@ export class NormalizeDelegate
declare readonly meta: { fetchedAt: number; date: number; expiresAt: number };
declare checkLoop: (entityKey: string, pk: string, input: object) => boolean;

protected newEntities = new Map<string, Map<string, any>>();
protected newIndexes = new Map<string, Map<string, any>>();
protected new = {
entities: new Map<string, Map<string, any>>(),
indexes: new Map<string, Map<string, any>>(),
};

constructor(
state: {
Expand All @@ -46,7 +58,15 @@ export class NormalizeDelegate
actionMeta: { fetchedAt: number; date: number; expiresAt: number },
) {
super(state);
this.entitiesMeta = state.entitiesMeta;
// this.normalized = NormalizedSchema<E, R> = {
// result: '' as any,
// entities: { ...state.entities },
// indexes: { ...state.indexes },
// entitiesMeta: { ...state.entitiesMeta },
// };
Copy link
Preview

Copilot AI Aug 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This large block of commented code should be removed as it adds clutter and doesn't provide value. If this is temporary implementation notes, consider using TODO comments instead.

Suggested change
// };
// TODO: Assign to this.normalized if/when needed.

Copilot uses AI. Check for mistakes.

this.entities = { ...state.entities };
this.indexes = { ...state.indexes };
this.entitiesMeta = { ...state.entitiesMeta };
this.meta = actionMeta;
this.checkLoop = getCheckLoop();
}
Expand All @@ -57,8 +77,8 @@ export class NormalizeDelegate

protected getNewEntities(key: string): Map<string, any> {
// first time we come across this type of entity
if (!this.newEntities.has(key)) {
this.newEntities.set(key, new Map());
if (!this.new.entities.has(key)) {
this.new.entities.set(key, new Map());
// we will be editing these, so we need to clone them first
this.entities[key] = {
...this.entities[key],
Expand All @@ -68,15 +88,15 @@ export class NormalizeDelegate
};
}

return this.newEntities.get(key) as Map<string, any>;
return this.new.entities.get(key) as Map<string, any>;
}

protected getNewIndexes(key: string): Map<string, any> {
if (!this.newIndexes.has(key)) {
this.newIndexes.set(key, new Map());
if (!this.new.indexes.has(key)) {
this.new.indexes.set(key, new Map());
this.indexes[key] = { ...this.indexes[key] };
}
return this.newIndexes.get(key) as Map<string, any>;
return this.new.indexes.get(key) as Map<string, any>;
}

/** Updates an entity using merge lifecycles when it has previously been set */
Expand Down
16 changes: 7 additions & 9 deletions packages/normalizr/src/normalize/normalize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,13 @@ See https://dataclient.io/rest/api/RestEndpoint#parseResponse for more informati
}
}

const ret: NormalizedSchema<E, R> = {
result: '' as any,
entities: { ...entities },
indexes: { ...indexes },
entitiesMeta: { ...entitiesMeta },
};
const visit = getVisit(new NormalizeDelegate(ret, meta));
ret.result = visit(schema, input, input, undefined, args);
return ret;
const delegate = new NormalizeDelegate(
{ entities, indexes, entitiesMeta },
meta,
);
const visit = getVisit(delegate);
delegate.result = visit(schema, input, input, undefined, args);
return delegate as any;
Copy link
Preview

Copilot AI Aug 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using 'as any' type assertion bypasses TypeScript's type checking. Consider defining proper type annotations or ensuring the delegate properly implements the expected return type to maintain type safety.

Suggested change
return delegate as any;
return {
result: delegate.result,
entities: delegate.entities,
indexes: delegate.indexes,
entitiesMeta: delegate.entitiesMeta,
};

Copilot uses AI. Check for mistakes.

};

function expectedSchemaType(schema: Schema) {
Expand Down