Skip to content

Conversation

@rekmarks
Copy link
Member

@rekmarks rekmarks commented Mar 28, 2025

Adds a new struct exactOptional that enables strictly optional properties on object structs.

When used with object() structs, { foo: optional(x()) } results in { foo?: x | undefined }. This makes optional() incompatible with e.g. the Json type of @metamask/utils. Using exactOptional(x()), we instead get exactly optional properties i.e. { foo?: x }.

The name strictOptional was previously considered, however exactOptional is in line with the exactOptionalPropertyTypes TypeScript configuration option, whose effect we are trying to achieve.

This implementation is superior to the one currently in @metamask/utils in two ways:

  1. The exactOptional struct and types should be forward-compatible with all versions of @metamask/superstruct.
  2. By modifying the object struct in @metamask/superstruct, we avoid creating a second struct that supports exactOptional.

@rekmarks rekmarks requested a review from a team March 28, 2025 17:22
@rekmarks rekmarks changed the title feat: Add strictOptional struct feat: Add strictOptional struct Mar 28, 2025
@rekmarks rekmarks enabled auto-merge (squash) March 28, 2025 17:23
@rekmarks rekmarks disabled auto-merge March 28, 2025 17:23
@rekmarks rekmarks enabled auto-merge (squash) March 28, 2025 17:23
@rekmarks rekmarks force-pushed the rekm/strict-optional branch from 8982082 to 189f8e3 Compare March 28, 2025 17:24
@rekmarks
Copy link
Member Author

Already exists in the form of exactOptional() in @metamask/utils.

@rekmarks rekmarks closed this Mar 28, 2025
auto-merge was automatically disabled March 28, 2025 17:34

Pull request was closed

@rekmarks rekmarks deleted the rekm/strict-optional branch March 28, 2025 17:34
@rekmarks rekmarks restored the rekm/strict-optional branch March 28, 2025 17:39
@rekmarks
Copy link
Member Author

We may still want this.

@rekmarks rekmarks reopened this Mar 28, 2025
Copy link
Member Author

Choose a reason for hiding this comment

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

I moved some stuff around in here but the only new thing is StrictOptionalize and its use in ObjectType.

@rekmarks rekmarks force-pushed the rekm/strict-optional branch from bd1cdb4 to cf1bdf7 Compare March 28, 2025 18:53
@rekmarks rekmarks changed the title feat: Add strictOptional struct feat: Add exactOptional struct Mar 28, 2025
@rekmarks rekmarks force-pushed the rekm/strict-optional branch from 19e1f15 to 72b1e00 Compare March 28, 2025 21:05
@rekmarks rekmarks requested a review from hmalik88 March 28, 2025 23:03
@FrederikBolding
Copy link
Member

Why do we wanna do this at the fork level instead of just using what already exists in @metamask/utils?

@rekmarks rekmarks force-pushed the rekm/strict-optional branch from a505589 to 368778e Compare March 31, 2025 20:22
@rekmarks rekmarks requested review from MajorLift and Mrtenz March 31, 2025 20:23
@rekmarks
Copy link
Member Author

@FrederikBolding updated description per our discussion.

Type = unknown,
Schema = unknown,
> extends Struct<Type, Schema> {
// eslint-disable-next-line no-restricted-syntax
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a comment to clarify why this is necessary?

Copy link
Member Author

Choose a reason for hiding this comment

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

@rekmarks rekmarks requested a review from Mrtenz April 1, 2025 16:43
@rekmarks rekmarks enabled auto-merge (squash) April 1, 2025 19:10
@rekmarks rekmarks merged commit db04d0b into main Apr 1, 2025
21 checks passed
@rekmarks rekmarks deleted the rekm/strict-optional branch April 1, 2025 20:37
rekmarks added a commit to MetaMask/utils that referenced this pull request Apr 2, 2025
Deprecates the local `exactOptional` implementation in favor of the one
from `@metamask/superstruct@3.2.0`. See:
MetaMask/superstruct#32
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.

5 participants