fix: Faster Assign type #2594
fix: Faster Assign type #2594
Conversation
🦋 Changeset detectedLatest commit: 5ae1c61 The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@likewerealive is attempting to deploy a commit to the Chakra UI Team on Vercel. A member of the Team first needs to authorize it. |
|
I'm unsure of the fixes in playground here - I saw that it had the assign type from a generation, so i ran |
|
hey, sorry for the answer delay ! I'll look into this PR asap, I mostly need to benchmark stuff as this impact a lot of types indirectly |
|
tldr: so, I spent a fair amount of time on this and it doesn't seem like this makes it faster, on the contrary Benchmark with @arktype/attestTesting with
import { bench } from '@arktype/attest'
import { cva } from '../styled-system/css'
import { styled } from '../styled-system/jsx'
import { HTMLStyledProps, RecipeVariantProps } from '../styled-system/types'
bench('Omit', () => {
const buttonRecipe = cva({
base: {
borderRadius: 'md',
fontWeight: 'semibold',
h: '10',
px: '4',
},
variants: {
visual: {
solid: {
bg: { base: 'colorPalette.500', _dark: 'colorPalette.300' },
color: { base: 'white', _dark: 'gray.800' },
},
outline: {
border: '1px solid',
color: { base: 'colorPalette.600', _dark: 'colorPalette.200' },
borderColor: 'currentColor',
},
},
},
defaultVariants: {
visual: 'solid',
},
})
type ButtonProps = HTMLStyledProps<'button'> &
RecipeVariantProps<typeof buttonRecipe> & {
isLoading?: boolean
}
const StyledButton = styled('button', buttonRecipe)
const Button = ({ isLoading, children, ...props }: ButtonProps) => {
return <StyledButton {...props}>{isLoading ? 'Loading' : children}</StyledButton>
}
const ExtendedButton = styled(Button, {
variants: {
isSpecial: {
true: {
bg: 'red.500',
},
},
},
})
return {} as any as typeof ExtendedButton
}).types([58034, 'instantiations'])
"bench": "bun run ./__tests__/types.bench.tsx",
// 1
// export type Assign<T, U> = {
// [K in keyof T]: K extends keyof U ? U[K] : T[K]
// } & U
// 2
// export type Assign<T, U> = DistributiveOmit<T, keyof U> & U
// 3
export type Assign<T, U> = Omit<T, keyof U> & UThis is the result, in the above order (1 = Original, 2 = DistributeOmit, 3 = Omit): panda/sandbox/codegen main* ≡3s
❯ bun bench
$ bun run ./__tests__/types.bench.tsx
🏌️ Omit
⛳ Result: 58034 instantiations
🎯 Baseline: 58034 instantiations
📊 Delta: 0.00%
panda/sandbox/codegen main* ≡3s
❯ bun bench
$ bun run ./__tests__/types.bench.tsx
🏌️ Omit
⛳ Result: 202002 instantiations
🎯 Baseline: 58034 instantiations
📈 'Omit' exceeded baseline by 248.08% (threshold is 20%).
panda/sandbox/codegen main* ≡2s
❯ bun bench
$ bun run ./__tests__/types.bench.tsx
🏌️ Omit
⛳ Result: 235239 instantiations
🎯 Baseline: 58034 instantiations
📈 'Omit' exceeded baseline by 305.35% (threshold is 20%).It seems like the original (what we currently have in Panda) leads to less type instantiations Typescript analyze-traceSince the above benchmark is an isolated case, which could be far from the real-world, I thought of trying an actual So, in the park-ui repo I went in the bun tsc -b --generateTrace trace-dir
bun x @typescript/analyze-trace trace-dir --forceMillis=100 --skipMillis=50Below is a summary of the 3 runs (I removed most of the details): name = Assign 1 (Original): report = Check file /website/src/components/demos/tags-input.demo.tsx (265ms) name = Assign 2 (DistributiveOmit): report = Check file /website/src/components/ui/avatar.tsx (435ms) name = Assign 3 (Omit) report = Check file /website/src/components/ui/badge.tsx (348ms) Markdown Table of Time Taken for Each FileI asked gpt to make a pretty markdown table with those infos, here it is:
ConclusionIt seems what we currently have is the fastest of the 3, even though the proposed alternatives can be faster in some cases (like yours ?), so I think we can't merge this PR, unless I made a mistake somewhere |
Hey @astahmer ! Thanks so much for the detailed response here! I couldn't get attest working in the short time i looked into it, but your code examples here have actually cleared that up for me! |
📝 Description
Update
Assigntype to useOmitto speed up type checking⛳️ Current behavior (updates)
The current Assign can be very slow when type checking
🚀 New behavior
Assign is much faster. This mimics the type from ark-ui
💣 Is this a breaking change (Yes/No):
No
📝 Additional Information
If we have a ui-lib with a panda button component like:
And we want to add some special variants to a single use-case of this Button in one product, we can do:
This runs fine in the app, but when it comes to type checking this hangs for a long time. I've found that updating the Assign type to mimic Ark-UI fixes this! I had a file that only contained an override, like the above, take
151,008.080msto type check. With this fix it took162.014ms