anatine / zod-plugins

Plugins and utilities for Zod
591 stars 84 forks source link

Proposal(zod-openapi): reduce build/bundle size by ~60kb by only type importing zod #163

Open tefkah opened 9 months ago

tefkah commented 9 months ago

Hi!

Love this package, have been using it successfully in several projects, thank you so much for making it!

TLDR: Removing instanceof and just importing zod as 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-rest or anything else that lets this into the bundle the size of the bundle increases dramatically because of the direct dependency on zod. Of course, anything that lets your packages into the bundle would probably also let zod into it, but because your package also explicitly imports zod/lib/types it confounds the issue.

I noticed that, barring 4 uses of instanceof, almost all the usage of zod in this package is on the type level. If we were able to import type instead of plain import zod, we would be able to drop zod from the js import, 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 for z.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 zod schemas 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 instanceof is 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 replace instanceof here. Would love your thoughts!

Comparisons

Base package

Before

image

This PR

image

Real world use case (pubpub-client)

Before

image

Using this PR

image
nx-cloud[bot] commented 9 months ago

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 3dd903b47e3ccad2e40470409922b800b1b2da5f. 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 targets - [`nx affected:test --base=origin/main --codeCoverage`](https://cloud.nx.app/runs/XcUuHGetws) - [`nx affected:lint --base=origin/main`](https://cloud.nx.app/runs/4YGoOhVjgD)

Sent with πŸ’Œ from NxCloud.

tefkah commented 9 months ago

I managed to fix the test.