-
Notifications
You must be signed in to change notification settings - Fork 27
Multi-package setup: types/zod + legacy wrapper #277
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4bfdb1111
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
packages/types/package.json
Outdated
| "scripts": { | ||
| "generate": "node scripts/emit-types.ts", | ||
| "lint:ts": "yarn generate && tsc --build tsconfig.build.json", | ||
| "test:unit": "node scripts/emit-types.test.ts", | ||
| "build": "yarn generate && tsc --build tsconfig.build.json", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a publish-time build hook for @transloadit/types
In packages/types/package.json, the scripts block defines generate/build but there is no prepack/prepare hook to ensure dist exists at publish time. Because dist/ is gitignored and the release workflow now runs changeset publish without a separate build step, publishing will produce a tarball that omits the generated .d.ts files, breaking consumers of @transloadit/types. Consider adding a prepack/prepare (or a release build step) that runs yarn generate && tsc --build before publishing.
Useful? React with 👍 / 👎.
packages/zod/package.json
Outdated
| "scripts": { | ||
| "sync:v3": "node scripts/sync-v3.ts", | ||
| "sync:v4": "node scripts/sync-v4.ts", | ||
| "sync": "yarn sync:v3 && yarn sync:v4", | ||
| "lint:ts": "yarn sync && tsc --build tsconfig.build.json", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a publish-time build hook for @transloadit/zod
In packages/zod/package.json, the scripts block only defines sync/build tasks, but there is no prepack/prepare hook to ensure dist exists when publishing. Since dist/ is not tracked and the release workflow now calls changeset publish without a build step, @transloadit/zod would be published without its compiled dist/v3 and dist/v4 exports. Add a publish-time build hook or run yarn sync && tsc --build in the release workflow before publishing.
Useful? React with 👍 / 👎.
This introduces:
refs #263 (as indicated zod v3 schemas remain our single source of truth, while much of the ecosystem has not switched at, it is synced with many other repos, and it would be harder to write transformers from v4 to v3 because some features are just not supported. The best time is when the majority of the js ecosystem is no longer on v3)
this paves the way for