Skip to content

Conversation

@nikgraf
Copy link
Collaborator

@nikgraf nikgraf commented Jul 25, 2025

No description provided.

@nikgraf nikgraf changed the title Ng/add type optional add type optional Jul 25, 2025
@nikgraf nikgraf force-pushed the ng/add-type-optional branch from 164455b to 6aee811 Compare July 28, 2025 17:03
@nikgraf nikgraf force-pushed the ng/add-type-optional branch from 6aee811 to 3cb14c4 Compare July 28, 2025 17:31
@nikgraf nikgraf force-pushed the ng/add-type-optional branch from 3cb14c4 to 89c3486 Compare July 28, 2025 17:32
@nikgraf nikgraf force-pushed the ng/add-type-optional branch from 89c3486 to 53937ce Compare July 28, 2025 17:41
@nikgraf nikgraf force-pushed the ng/add-type-optional branch from 53937ce to 343fd95 Compare July 28, 2025 17:43
@nikgraf nikgraf force-pushed the ng/add-type-optional branch from 343fd95 to 9902a6e Compare July 28, 2025 17:50
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for optional fields in the hypergraph schema system by introducing a Type.optional function and utilities to handle optional type checking throughout the codebase.

  • Adds Type.optional export and comprehensive type utilities for detecting optional fields
  • Updates serialization logic to handle optional fields by skipping undefined values or throwing errors for required fields
  • Demonstrates the feature with optional fields in schema examples and practical usage in the events app

Reviewed Changes

Copilot reviewed 17 out of 18 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
packages/hypergraph/src/type/type.ts Exports Schema.optional as Type.optional
packages/hypergraph/src/type-utils/type-utils.ts Adds utility functions to detect optional and specific optional field types
packages/hypergraph/src/index.ts Exports the new TypeUtils module
packages/hypergraph-react/test/prepare-publish.test.ts Comprehensive tests for the preparePublish function with optional field handling
packages/hypergraph-react/src/use-query.tsx Removes commented-out legacy code for merging entities
packages/hypergraph-react/src/prepare-publish.ts Updates to use TypeUtils for field type checking and optional field handling
packages/hypergraph-react/src/internal/use-query-public.tsx Updates property value conversion to use TypeUtils
packages/hypergraph-react/src/internal/use-generate-update-ops.tsx Removes entire file (was commented out)
packages/hypergraph-react/src/internal/use-create-entity-public.ts Updates to use TypeUtils and handle optional fields
docs/docs/schema.md Adds documentation for optional fields
apps/typesync/src/Generator.ts Minor import ordering change
apps/events/src/schema.ts Adds optional description field to Event schema
apps/events/src/mapping.ts Adds mapping for the new description property
apps/events/src/components/ui/label.tsx Adds new Label UI component
apps/events/src/components/playground.tsx Increases query limit from 2 to 10
apps/events/src/components/events/events.tsx Updates UI to handle optional description field
apps/events/package.json Adds @radix-ui/react-label dependency
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

Comment on lines 5 to 39
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Text;
}
return type === Type.Text;
};

// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isNumberOrOptionalNumberType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Number;
}
return type === Type.Number;
};

// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isDateOrOptionalDateType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Date;
}
return type === Type.Date;
};

// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isCheckboxOrOptionalCheckboxType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Checkbox;
}
return type === Type.Checkbox;
};

// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isPointOrOptionalPointType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Point;
}
Copy link

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

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

The function isStringOrOptionalStringType returns type === Type.Text in both branches of the conditional, making the conditional logic redundant. Consider simplifying this to just return type === Type.Text; or implementing proper optional type checking.

Suggested change
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Text;
}
return type === Type.Text;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isNumberOrOptionalNumberType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Number;
}
return type === Type.Number;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isDateOrOptionalDateType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Date;
}
return type === Type.Date;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isCheckboxOrOptionalCheckboxType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Checkbox;
}
return type === Type.Checkbox;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isPointOrOptionalPointType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Point;
}
return type === Type.Text;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isNumberOrOptionalNumberType = (type: any) => {
return type === Type.Number;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isDateOrOptionalDateType = (type: any) => {
return type === Type.Date;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isCheckboxOrOptionalCheckboxType = (type: any) => {
return type === Type.Checkbox;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isPointOrOptionalPointType = (type: any) => {

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess I don't disagree with the copilot response here that both branches are the same. Is there a reason for if the if check if it only returns the same as the else case?

Comment on lines 5 to 39
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Text;
}
return type === Type.Text;
};

// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isNumberOrOptionalNumberType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Number;
}
return type === Type.Number;
};

// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isDateOrOptionalDateType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Date;
}
return type === Type.Date;
};

// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isCheckboxOrOptionalCheckboxType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Checkbox;
}
return type === Type.Checkbox;
};

// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isPointOrOptionalPointType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Point;
}
Copy link

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

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

The function isNumberOrOptionalNumberType has the same logic issue as isStringOrOptionalStringType - both branches return the same value, making the conditional redundant.

Suggested change
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Text;
}
return type === Type.Text;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isNumberOrOptionalNumberType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Number;
}
return type === Type.Number;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isDateOrOptionalDateType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Date;
}
return type === Type.Date;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isCheckboxOrOptionalCheckboxType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Checkbox;
}
return type === Type.Checkbox;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isPointOrOptionalPointType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Point;
}
return type === Type.Text;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isNumberOrOptionalNumberType = (type: any) => {
return type === Type.Number;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isDateOrOptionalDateType = (type: any) => {
return type === Type.Date;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isCheckboxOrOptionalCheckboxType = (type: any) => {
return type === Type.Checkbox;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isPointOrOptionalPointType = (type: any) => {

Copilot uses AI. Check for mistakes.
Comment on lines 5 to 39
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Text;
}
return type === Type.Text;
};

// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isNumberOrOptionalNumberType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Number;
}
return type === Type.Number;
};

// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isDateOrOptionalDateType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Date;
}
return type === Type.Date;
};

// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isCheckboxOrOptionalCheckboxType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Checkbox;
}
return type === Type.Checkbox;
};

// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isPointOrOptionalPointType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Point;
}
Copy link

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

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

The function isDateOrOptionalDateType has the same logic issue - both branches return the same value, making the conditional redundant.

Suggested change
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Text;
}
return type === Type.Text;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isNumberOrOptionalNumberType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Number;
}
return type === Type.Number;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isDateOrOptionalDateType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Date;
}
return type === Type.Date;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isCheckboxOrOptionalCheckboxType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Checkbox;
}
return type === Type.Checkbox;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isPointOrOptionalPointType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Point;
}
return type === Type.Text;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isNumberOrOptionalNumberType = (type: any) => {
return type === Type.Number;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isDateOrOptionalDateType = (type: any) => {
return type === Type.Date;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isCheckboxOrOptionalCheckboxType = (type: any) => {
return type === Type.Checkbox;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isPointOrOptionalPointType = (type: any) => {

Copilot uses AI. Check for mistakes.
Comment on lines 5 to 39
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Text;
}
return type === Type.Text;
};

// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isNumberOrOptionalNumberType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Number;
}
return type === Type.Number;
};

// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isDateOrOptionalDateType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Date;
}
return type === Type.Date;
};

// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isCheckboxOrOptionalCheckboxType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Checkbox;
}
return type === Type.Checkbox;
};

// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isPointOrOptionalPointType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Point;
}
Copy link

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

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

The function isCheckboxOrOptionalCheckboxType has the same logic issue - both branches return the same value, making the conditional redundant.

Suggested change
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Text;
}
return type === Type.Text;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isNumberOrOptionalNumberType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Number;
}
return type === Type.Number;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isDateOrOptionalDateType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Date;
}
return type === Type.Date;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isCheckboxOrOptionalCheckboxType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Checkbox;
}
return type === Type.Checkbox;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isPointOrOptionalPointType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Point;
}
return type === Type.Text;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isNumberOrOptionalNumberType = (type: any) => {
return type === Type.Number;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isDateOrOptionalDateType = (type: any) => {
return type === Type.Date;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isCheckboxOrOptionalCheckboxType = (type: any) => {
return type === Type.Checkbox;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isPointOrOptionalPointType = (type: any) => {

Copilot uses AI. Check for mistakes.
Comment on lines 5 to 39
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Text;
}
return type === Type.Text;
};

// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isNumberOrOptionalNumberType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Number;
}
return type === Type.Number;
};

// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isDateOrOptionalDateType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Date;
}
return type === Type.Date;
};

// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isCheckboxOrOptionalCheckboxType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Checkbox;
}
return type === Type.Checkbox;
};

// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isPointOrOptionalPointType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Point;
}
Copy link

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

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

The function isPointOrOptionalPointType has the same logic issue - both branches return the same value, making the conditional redundant.

Suggested change
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Text;
}
return type === Type.Text;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isNumberOrOptionalNumberType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Number;
}
return type === Type.Number;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isDateOrOptionalDateType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Date;
}
return type === Type.Date;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isCheckboxOrOptionalCheckboxType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Checkbox;
}
return type === Type.Checkbox;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isPointOrOptionalPointType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Point;
}
return type === Type.Text;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isNumberOrOptionalNumberType = (type: any) => {
return type === Type.Number;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isDateOrOptionalDateType = (type: any) => {
return type === Type.Date;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isCheckboxOrOptionalCheckboxType = (type: any) => {
return type === Type.Checkbox;
};
// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isPointOrOptionalPointType = (type: any) => {

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

@cmwhited cmwhited left a comment

Choose a reason for hiding this comment

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

Check comment on is{Type}OrOptional{Type}Type fn

Comment on lines 5 to 39
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Text;
}
return type === Type.Text;
};

// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isNumberOrOptionalNumberType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Number;
}
return type === Type.Number;
};

// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isDateOrOptionalDateType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Date;
}
return type === Type.Date;
};

// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isCheckboxOrOptionalCheckboxType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Checkbox;
}
return type === Type.Checkbox;
};

// biome-ignore lint/suspicious/noExplicitAny: TODO
export const isPointOrOptionalPointType = (type: any) => {
if (type.ast && type.ast._tag === 'PropertySignatureDeclaration' && type.ast.isOptional) {
return type === Type.Point;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess I don't disagree with the copilot response here that both branches are the same. Is there a reason for if the if check if it only returns the same as the else case?

@nikgraf
Copy link
Collaborator Author

nikgraf commented Jul 30, 2025

@cmwhited excellent catch! fixed it and added tests for more optional cases

@nikgraf nikgraf merged commit f8ccaed into main Jul 30, 2025
6 checks passed
@nikgraf nikgraf deleted the ng/add-type-optional branch July 30, 2025 17:57
@nikgraf nikgraf mentioned this pull request Jul 30, 2025
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.

3 participants