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

BRAND Record to Non Partial #2097

Merged
merged 1 commit into from
Feb 26, 2023

Conversation

Hiroshiba
Copy link
Contributor

This PR is an artifact of the discussion.
#2069 (reply in thread)

A Record keyed by string, number or symbol is Non Partial, but BRAND (which should behave similarly) is Partial.
This PR ensures that a key with BRAND will also be Non Partial.

@netlify
Copy link

netlify bot commented Feb 23, 2023

Deploy Preview for guileless-rolypoly-866f8a ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit e9298e2
🔍 Latest deploy log https://app.netlify.com/sites/guileless-rolypoly-866f8a/deploys/63f7e6ce12180800081a2781
😎 Deploy Preview https://deploy-preview-2097--guileless-rolypoly-866f8a.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@colinhacks colinhacks merged commit 8f3d028 into colinhacks:master Feb 26, 2023
@colinhacks
Copy link
Owner

Looks good!

@Hiroshiba Hiroshiba deleted the brand-record-to-nonpartial branch February 26, 2023 22:35
@wdanilo
Copy link

wdanilo commented Feb 8, 2024

It doesn't work:

const zodKey = Z.string().brand('zodKey')
const zodRecord = Z.record(zodKey, Z.number())
type ZodRecord = Z.infer<typeof zodRecord>

still infers as possibly undefined values. Is there maybe a regression @colinhacks? @Hiroshiba I don't see any tests in your PR, was it working with this example when you were submitting it?

@fwoelffel
Copy link

I am also observing this unexpected behaviour where using a branded type as the record key would result in a partial record 🤔

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