Skip to content

feat: standard schema for validation#543

Merged
dreyfus92 merged 14 commits into
bombshell-dev:mainfrom
florian-lefebvre:feat/standard-schema
May 28, 2026
Merged

feat: standard schema for validation#543
dreyfus92 merged 14 commits into
bombshell-dev:mainfrom
florian-lefebvre:feat/standard-schema

Conversation

@florian-lefebvre

@florian-lefebvre florian-lefebvre commented May 19, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Allows using Standard Schema compatible libraries for validate(). Discussed on Discord.

Docs PR: bombshell-dev/docs#42

Type of change

  • Bug fix
  • Feature
  • Refactor (no behavior change)
  • Documentation
  • Performance improvement
  • Tests
  • Chore (dependencies, CI, tooling)

Checklist

  • pnpm test passes (or targeted tests for my change)
  • pnpm format has been run
  • I have added/updated tests for my changes (if applicable)
  • I have added a changeset

AI-generated code disclosure

  • This PR includes AI-generated code

@changeset-bot

changeset-bot Bot commented May 19, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: f1419b5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@clack/prompts Minor
@clack/core Minor

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

@pkg-pr-new

pkg-pr-new Bot commented May 19, 2026

Copy link
Copy Markdown

commit: f1419b5

@ghostdevv ghostdevv left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the jsdoc comments above the validate options would need updating, as well as the docs https://github.com/bombshell-dev/docs/

Comment thread examples/basic/standard-schema-validation.ts Outdated
Comment thread packages/core/src/utils/validation.ts

@natemoo-re natemoo-re left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks great, changeset is perfect, just minor feedback on the example!

Comment thread examples/basic/standard-schema-validation.ts Outdated
Comment thread packages/core/src/utils/validation.ts Outdated
@florian-lefebvre

florian-lefebvre commented May 20, 2026

Copy link
Copy Markdown
Contributor Author

I also want to note that the PR does not support schema transforms (they will run but not be used). That would require more changes to represent input and output in types. Out of scope of this PR but if you're interested let me know!

@florian-lefebvre

Copy link
Copy Markdown
Contributor Author

@ghostdevv docs PR is available at bombshell-dev/docs#42

@ghostdevv

Copy link
Copy Markdown
Member

Also, this would be breaking right?

Comment thread packages/core/src/utils/validation.ts Outdated
Comment thread packages/core/src/utils/validation.ts Outdated
Comment thread packages/core/package.json Outdated
florian-lefebvre and others added 3 commits May 20, 2026 16:48
Co-authored-by: James Garbutt <43081j@users.noreply.github.com>
Co-authored-by: James Garbutt <43081j@users.noreply.github.com>
@florian-lefebvre

Copy link
Copy Markdown
Contributor Author

@43081j yeah I was also hesitating. I inlined it, the spec says it's absolutely fine

@florian-lefebvre

Copy link
Copy Markdown
Contributor Author

Also, this would be breaking right?

@ghostdevv by "this", are you referring to input/output/transforms or this PR?

@ghostdevv

Copy link
Copy Markdown
Member

Also, this would be breaking right?

@ghostdevv by "this", are you referring to input/output/transforms or this PR?

Yea, as this PR changes the behavior of the validate option

@florian-lefebvre

florian-lefebvre commented May 20, 2026

Copy link
Copy Markdown
Contributor Author

I'm not sure it's fully breaking. No change as a user but it's true that if you are dealing with lower level APIs like in the date prompt, then it requires changes. So maybe it's a major for core and a minor for prompts

@ghostdevv

Copy link
Copy Markdown
Member

I'm not sure it's fully breaking. No change as a user but it's true that if you are dealing with lower level APIs like in the date prompt, then it requires changes. So maybe it's a major for core and a minor for prompts

Ah, I'm so sorry but I missed the fact that the old behavior is still available 🤦‍♀️

@43081j

43081j commented May 21, 2026

Copy link
Copy Markdown
Collaborator

technically, we can strip the standard schema types down to only the parts we use too. but i'm happy either way.

inlining it is better than pulling a dependency still

@florian-lefebvre

Copy link
Copy Markdown
Contributor Author

I just checked and very little can be stripped, only InferInput and InferOutput. So I'd say it's worth leaving it as is

Comment thread examples/basic/standard-schema-validation.ts Outdated
Comment thread packages/core/src/prompts/prompt.ts Outdated
Comment thread packages/core/src/utils/validation.ts Outdated
florian-lefebvre and others added 3 commits May 22, 2026 09:40
Co-authored-by: Willow (GHOST) <ghostdevbusiness@gmail.com>
Co-authored-by: Willow (GHOST) <ghostdevbusiness@gmail.com>

@ghostdevv ghostdevv left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

last thing 😄 otherwise lgtm

Comment thread packages/core/src/utils/validation.ts
Co-authored-by: Willow (GHOST) <ghostdevbusiness@gmail.com>

@dreyfus92 dreyfus92 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you @florian-lefebvre 💜

@gameroman gameroman requested a review from ghostdevv May 27, 2026 19:18
@dreyfus92 dreyfus92 merged commit 83428ac into bombshell-dev:main May 28, 2026
7 checks passed
@bombshell-bot bombshell-bot Bot mentioned this pull request May 29, 2026
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.

6 participants