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

fix: tree shake named exports #2850

Merged
merged 5 commits into from
Apr 24, 2024
Merged

Conversation

olehmisar
Copy link

@olehmisar olehmisar commented Oct 8, 2023

This PR allows vite/esbuild to tree shake unused exports if user imports schemas using named exports:

import { number } from 'zod';

Before this PR, vite will bundle the whole library regardless what schemas you use. Now, it will bundle only what is needed. E.g., if you import only number, vite will not include string in the final bundle.

NOTE: Compiling ESM with tsc would also treeshake import { z } from 'zod'. This is not included in this PR but I believe this would be highly desirable.

Closes #2596.

How this is fixed

  • For some reason, static create = (...) => { ... } is not treeshakeable by vite/esbuild, but static create(...) { ... } is.
  • Using const numberType = ZodNumber.create is not treeshakeable but const numberType = (...args) => ZodNumber.create(...args) is.
  • Extracted coerce to a separate file and used named export for each schema instead of export const coerce.
  • Removed export namespace exports because the code that is generated by typescript is not treeshakeable.

To further improve tree shaking

  • breaking change: consider removing convenience methods from ZodType. Even with this PR merged, a single number() produces a 1600 line file. It includes ZodArray because of .array(), ZodOptional because of .optional(), etc.
    • Removing all convenience methods from ZodType still produces 900 lines of code in final bundle (vs 1600 lines with convenience methods). Most of that code (600 lines) is related to ZodError

@netlify
Copy link

netlify bot commented Oct 8, 2023

Deploy Preview for guileless-rolypoly-866f8a ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 4e6c85f
🔍 Latest deploy log https://app.netlify.com/sites/guileless-rolypoly-866f8a/deploys/662851745ae9e000098f0156
😎 Deploy Preview https://deploy-preview-2850--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 configuration.

@dreamorosi
Copy link

Hi, thanks for this PR.

I wanted to understand if there's any plan on reviewing and/or merging it - @colinhacks

@AlanLes
Copy link

AlanLes commented Mar 12, 2024

Bumping this up as this is a really wanted feature! 🤞😇

@ekwoka
Copy link

ekwoka commented Mar 12, 2024

I'd imagine static create = isn't treeshakeable because it's an actual runtime expression, as opposed to a simple declaration. Declarations won't have side effects, but expressions can.

@olehmisar
Copy link
Author

A note: the last commit "make util tree shakeable" is a breaking change. I should probably remove it or make a separate PR for it.

@colinhacks
Copy link
Owner

This is some really great work but I was afraid to merge this in a minor release. There's also much more to be done to improve treeshaking that I'll be doing as part of a big Zod 4 refactory. I'd like to merge this as a starting point — @olehmisar can you un-archive your fork so I can merge?

@olehmisar olehmisar force-pushed the fix/tree-shaking branch 2 times, most recently from 6b1692b to 4e6c85f Compare April 24, 2024 00:25
@olehmisar
Copy link
Author

@colinhacks i rebased the branch on latest master. Also, i removed the util refactor, so this PR can be merged as a minor fix. I will open the util refactor as a separate PR if needed (see https://github.com/olehmisar/zod/tree/fix/tree-shaking-util)

@colinhacks colinhacks changed the base branch from master to v4 April 24, 2024 01:46
@colinhacks colinhacks merged commit dc6365c into colinhacks:v4 Apr 24, 2024
@colinhacks
Copy link
Owner

colinhacks commented Apr 24, 2024

Thanks @olehmisar! I ended up merging it with the util refactor — it was long overdue as well. I appreciate it a lot.

@olehmisar
Copy link
Author

@colinhacks what about this note?

Compiling ESM with tsc would also treeshake import { z } from 'zod'. This is not included in this PR but I believe this would be highly desirable.

Are you open for a PR for this feature?

This PR only improved treeshaking of import { number } from 'zod' and similar.

colinhacks added a commit to ytsunekawa/zod that referenced this pull request May 3, 2024
* fix: tree shake named exports

* docs: explain why wrapper functions

* fix: make coerce tree shakeable

* fix: make util tree shakeable

* Fix merge conflicts

---------

Co-authored-by: Colin McDonnell <colinmcd94@gmail.com>
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.

Bloating with esbuild bundled output
5 participants