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.

Sign up for GitHub

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

Jump to bottom

feat(scaffolder): add first class citizen support for zod #26957

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

feat(scaffolder): add first class citizen support for zod #26957

wants to merge 2 commits into from
+576 −101

Conversation

schultzp2020
Copy link
Contributor

Hey, I just made a Pull Request!

#26933

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)
  • All your commits have a Signed-off-by line in the message. (more info)

@schultzp2020 schultzp2020 requested review from a team as code owners October 3, 2024 19:30
@schultzp2020 schultzp2020 marked this pull request as draft October 3, 2024 19:30
@github-actions github-actions bot added the area:scaffolder Everything and all things related to the scaffolder project area label Oct 3, 2024
@backstage-goalie
Copy link
Contributor

backstage-goalie bot commented Oct 3, 2024 •
edited
Loading

Important

This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior.## Changed Packages

Package Name Package Path Changeset Bump Current Version
@backstage/types packages/types minor v1.2.0
@backstage/plugin-scaffolder-node plugins/scaffolder-node minor v0.6.2

@schultzp2020 schultzp2020 mentioned this pull request Oct 3, 2024
Open
2 tasks
@github-actions GitHub Actions
Copy link
Contributor

This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution!

@github-actions github-actions bot added the stale label Oct 17, 2024
@schultzp2020
Copy link
Contributor Author

Not stale

@github-actions github-actions bot removed the stale label Oct 18, 2024
@acierto acierto mentioned this pull request Oct 19, 2024
Merged
@schultzp2020
Copy link
Contributor Author

@benjdlambert @Rugvip PTAL?

benjdlambert
benjdlambert requested changes Oct 24, 2024
View reviewed changes
Copy link
Member

@benjdlambert benjdlambert left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Hey 👋 sorry it's taken a while to get round to this, have a million and other things to do as well. Some higher level comments that I think we want to address before moving this forwards.

The changes to support zod by default with the (z) => z.string() syntax for input and output schemas also aligns us pretty nicely with the new frontend system https://backstage.io/docs/frontend-system/architecture/extensions#extension-configuration and something that we're looking to rollout further around the project too.

plugins/scaffolder-node/src/actions/createScaffolderAction.ts Outdated Show resolved Hide resolved
plugins/scaffolder-node/src/actions/createTemplateAction.ts Outdated Show resolved Hide resolved
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 7, 2024

This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution!

@github-actions github-actions bot added the stale label Nov 7, 2024
@schultzp2020
Copy link
Contributor Author

Not stale, been busy for KubeCon.

@github-actions github-actions bot removed the stale label Nov 7, 2024
@benjdlambert
Copy link
Member

@schultzp2020 just to recap, we were chatting how we can get a zod schema from Record<string, ZodType> right?

Can we not just do something like:

const zod = z.object(
  Object.fromEntries(
    Object.entries(input).map(
      ([k, v]) => [k, v(z)]
     )
  )
)

Obviously for both input and output schemas, but that should allow us to use zod-to-json-schema to generate the view model, but all validation just goes through the zod object.

@schultzp2020
Copy link
Contributor Author

schultzp2020 commented Nov 15, 2024 •
edited
Loading

@schultzp2020 just to recap, we were chatting how we can get a zod schema from Record<string, ZodType> right?

Can we not just do something like:

const zod = z.object(
  Object.fromEntries(
    Object.entries(input).map(
      ([k, v]) => [k, v(z)]
     )
  )
)

Obviously for both input and output schemas, but that should allow us to use zod-to-json-schema to generate the view model, but all validation just goes through the zod object.

Thank you for the idea! I haven't thought of that, let me try it.

@schultzp2020 schultzp2020 marked this pull request as ready for review November 18, 2024 17:57
@schultzp2020
Copy link
Contributor Author

@benjdlambert After further investigation, I believe overloading createTemplateAction is not a good idea. The generic types differ significantly, causing numerous type errors. In the long term, maintaining an overloaded function would likely be far more challenging. Instead, it would be better to properly deprecate the old functionality and encourage the use of the new function.

@schultzp2020 schultzp2020 marked this pull request as draft November 18, 2024 22:12
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 2, 2024

This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution!

@github-actions github-actions bot added the stale label Dec 2, 2024
@schultzp2020
Copy link
Contributor Author

Not stale

@github-actions github-actions bot removed the stale label Dec 4, 2024
@schultzp2020
feat(scaffolder): add first class citizen support for zod
58f6cfd 
Signed-off-by: Paul Schultz <pschultz@pobox.com>

backport better typescript support

Signed-off-by: Paul Schultz <pschultz@pobox.com>

wip

Signed-off-by: Paul Schultz <pschultz@pobox.com>

wip

Signed-off-by: Paul Schultz <pschultz@pobox.com>

complete refactor

Signed-off-by: Paul Schultz <pschultz@pobox.com>
@schultzp2020
Copy link
Contributor Author

Updated the type names as suggested in the Scaffolder SIG.

@schultzp2020
updated naming schema
083246c 
Signed-off-by: Paul Schultz <pschultz@pobox.com>
@github-actions GitHub Actions
Copy link
Contributor

This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution!

@github-actions github-actions bot added the stale label Dec 31, 2024
@benjdlambert benjdlambert removed the stale label Jan 1, 2025
@github-actions GitHub Actions
Copy link
Contributor

This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution!

@github-actions github-actions bot added the stale label Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Rugvip Rugvip Awaiting requested review from Rugvip Rugvip is a code owner automatically assigned from backstage/maintainers

@camilaibs camilaibs Awaiting requested review from camilaibs camilaibs is a code owner automatically assigned from backstage/reviewers

@acierto acierto Awaiting requested review from acierto acierto is a code owner automatically assigned from backstage/scaffolder-maintainers

@benjdlambert benjdlambert Awaiting requested review from benjdlambert

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
area:scaffolder Everything and all things related to the scaffolder project area
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@schultzp2020 @benjdlambert