Skip to content
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

Mark graph object readonly #1122

Merged
merged 6 commits into from
Sep 19, 2024
Merged

Mark graph object readonly #1122

merged 6 commits into from
Sep 19, 2024

Conversation

VDubber
Copy link
Contributor

@VDubber VDubber commented Sep 19, 2024

When graph objects are iterated, mark them as Readonly.

Clean up unnecessary debugging logs.

- Updated the GraphObjectIteratee type to have a readonly entity parameter. When entities are being iterated in the graph store, they should never be modified. This change will indicate what integrations are breaking this pattern.
- Add GraphObjectStore test that demonstrates entities are readonly during iteration. This can be ignored to avoid TS errors.
@VDubber VDubber requested a review from a team as a code owner September 19, 2024 20:07
@@ -4,7 +4,7 @@ export interface GraphObjectFilter {
_type: string;
}

export type GraphObjectIteratee<T> = (obj: T) => void | Promise<void>;
export type GraphObjectIteratee<T> = (obj: Readonly<T>) => void | Promise<void>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd consider this change a major version bump. Agreed?

Copy link
Contributor Author

@VDubber VDubber Sep 19, 2024

Choose a reason for hiding this comment

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

I've decided that since this is enforcing a pre-existing understanding/pattern, it should only be a minor version bump.

gastonyelmini
gastonyelmini previously approved these changes Sep 19, 2024
@@ -506,6 +506,34 @@ describe('findEntity', () => {
});

describe('iterateEntities', () => {
test('iterated entities are mutable, but only if TS error is ignored.', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

😍

RonaldEAM
RonaldEAM previously approved these changes Sep 19, 2024
…d within this monorepo. This should improve ability to use alpha packages in the future.
@VDubber VDubber merged commit 885f102 into main Sep 19, 2024
8 checks passed
@VDubber VDubber deleted the readonly-iterate-graph-object branch September 19, 2024 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants