Proposal(zod-openapi): reduce build/bundle size by ~60kb by only type importing zod #163
Conversation
| zodRef._def.catchall instanceof z.ZodNever || | ||
| // zodRef._def.catchall instanceof z.ZodNever || |
There was a problem hiding this comment.
This assuredly misses some things, how could we check this without using instanceof?
There was a problem hiding this comment.
I actually don't think this misses things, if X is instanceof Y, it will definitely have the same typename.
Of course, things could theoretically have the same typename but not be the same zod type, but looking at zod this seems unlikely to happen.
| item instanceof z.ZodDefault || | ||
| // item instanceof z.ZodDefault || | ||
| item._def.typeName === 'ZodDefault' | ||
| ) && !(item instanceof z.ZodNever || item._def.typeName === 'ZodDefault') | ||
| ) && !( | ||
| // item instanceof z.ZodNever || | ||
| item._def.typeName === 'ZodNever' || | ||
| item._def.typeName === 'ZodDefault') |
| zodRef._def.valueType instanceof z.ZodUnknown | ||
| // @ts-expect-error This comparison appears to be unintentional because the types 'ZodFirstPartyTypeKind.ZodRecord' and '"ZodUnknown"' have no overlap.ts(2367) | ||
| zodRef._def.typeName === 'ZodUnknown' | ||
| // zodRef._def.valueType instanceof z.ZodUnknown |
There was a problem hiding this comment.
This one definitely does not work; zodRef._def.typeName does not work like this, I've had to ts-expect-error it. Do you know how to fix this?
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 3dd903b. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this branch ✅ Successfully ran 2 targetsSent with 💌 from NxCloud. |
tefkah
changed the title
tefkah
changed the title
|
I managed to fix the test. |
Hi!
Love this package, have been using it successfully in several projects, thank you so much for making it!
TLDR: Removing
instanceofand just importingzodas types can reduce bundle sizes in some cases by 60kb. I think this is a worthwile gain for relatively little pain.I've noticed that when used in combination with e.g.
ts-restor anything else that lets this into the bundle the size of the bundle increases dramatically because of the direct dependency onzod.Of course, anything that lets your packages into the bundle would probably also let
zodinto it, but because your package also explicitly importszod/lib/typesit confounds the issue.I noticed that, barring 4 uses of
instanceof, almost all the usage ofzodin this package is on the type level. If we were able toimport typeinstead of plainimportzod, we would be able to dropzodfrom thejsimport, which would be nice.So that is what I did, see below. I rather naively replaced the calls to
instanceof, which leads to one test failing (one that checks forz.record(z.unknown()). I don't really know how to fix this, hence this being a draft PR.See here a comparison of the different bundle sizes, checked by doing
npx banal dist/packages/zod-openapi/src/index.js.I've also tested this in a real world use case (https://github.com/tefkah/pubpub-client/tree/feat/integrate), using the dep from npm vs the built one in this PR. I can go into detail on how to do this, but the method is quite reliable (uses
yalc).As you can see, it saves a cool 60kb, nothing to sneeze at!
Of course, this whole problem can be solved by not including
zodschemas in the bundle in the first place, but sometimes this is either unavoidable or desirable (say you want to do client-side validation). Having to separate out the openapi comments can be a bit of a pain, and this PR could be a relatively cheap way save some kbs.Concrete questions
I've marked the 4 places where
instanceofis used and how I attempted to replace them. Could you have a look to see if this makes sense? At least one of them is not a valid replacement, and maybe it is impossible to truly replaceinstanceofhere. Would love your thoughts!Comparisons
Base package
Before
This PR
Real world use case (pubpub-client)
Before
Using this PR