Skip to content

Conversation

@jaredhill4
Copy link

@jaredhill4 jaredhill4 commented Jun 13, 2025

Closes #729.

With this change, developers can now configure their own custom field types like this:

fieldTypes: {
  image: ({ onChange }) => <input type="file" onChange={onChange} />,
},

I realize there are likely some type considerations, but it worked for me.


Demo

Screen.Recording.2025-06-12.at.10.52.05.PM.mov

@vercel
Copy link

vercel bot commented Jun 13, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
puck-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 14, 2025 11:03am
puck-docs ✅ Ready (Inspect) Visit Preview Jun 14, 2025 11:03am

@vercel
Copy link

vercel bot commented Jun 13, 2025

@jaredhill4 is attempting to deploy a commit to the Measured Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

@chrisvxd chrisvxd left a comment

Choose a reason for hiding this comment

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

This seems... too simple. I'll sanity check before merging.

@jaredhill4
Copy link
Author

This seems... too simple. I'll sanity check before merging.

Yea there were a couple considerations:

  1. UsingdefaultFields for children was what throwing the error when trying to use a custom field type in overrides.
  2. The overrides.fieldTypes is spread into render, so we should just use render from there down (unless the goal or thought was to error when trying to use custom field types).
  3. From what I could tell, passing children to Render seemed unnecessary.

@jaredhill4 jaredhill4 requested a review from chrisvxd June 26, 2025 18:36
@chrisvxd chrisvxd requested a review from matthewlynch July 7, 2025 14:39
Copy link
Contributor

@chrisvxd chrisvxd left a comment

Choose a reason for hiding this comment

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

I've done some more thinking on this, and it is indeed too good to be true.

You're removing children because it's getting in the way of the user field type specified in overrides, but the children is necessary as overrides expect the default node to be provided to the override as the children prop, which enables the user to wrap the node.

The correct fix for runtime behaviour would be to conditionally check the render function, or default to null (const children = defaultFields[field.type]?.(mergedProps) ?? null;).

However, the type issues are significant and require much further thought before any solution can be merged.

I'm going to close this one out for now whilst I think about the type issues.

Thanks!

@chrisvxd chrisvxd closed this Jul 9, 2025
@chrisvxd
Copy link
Contributor

chrisvxd commented Jul 9, 2025

New attempt at fix: #1147

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.

Custom field type overrides result in type error

2 participants