Skip to content

fix: invalid option error when modifying enums in docsSchema's extend#3936

Open
miichom wants to merge 15 commits into
withastro:mainfrom
miichom:main
Open

fix: invalid option error when modifying enums in docsSchema's extend#3936
miichom wants to merge 15 commits into
withastro:mainfrom
miichom:main

Conversation

@miichom

@miichom miichom commented May 31, 2026

Copy link
Copy Markdown

Description

In the starlight docs, it mentions docsSchema() can modify fields, however it is unable to modify z.enum fields The likely cause is from how ZodIntersection are treated.

Why this happens

The Zod documentation explicitly notes that intersections (a logical "AND") produce a ZodIntersection instance rather than a ZodObject.

From the Zod v4 docs:

When merging object schemas, prefer A.extend(B) over intersections. Using .extend() will give you a new object schema, whereas z.intersection(A, B) returns a ZodIntersection instance which lacks common object methods like pick and omit.

This limitation also affects any tooling that expects to operate on a ZodObject, including schema‑amending utilities like docsSchema().

@changeset-bot

changeset-bot Bot commented May 31, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: d58283d

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

This PR includes changesets to release 1 package
Name Type
@astrojs/starlight Patch

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

@netlify

netlify Bot commented May 31, 2026

Copy link
Copy Markdown

Deploy Preview for astro-starlight ready!

Name Link
🔨 Latest commit d58283d
🔍 Latest deploy log https://app.netlify.com/projects/astro-starlight/deploys/6a5faf9addbbbf0008c0e383
😎 Deploy Preview https://deploy-preview-3936--astro-starlight.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 92 (🔴 down 8 from production)
Accessibility: 100 (no change from production)
Best Practices: 100 (no change from production)
SEO: 100 (no change from production)
PWA: -
View the detailed breakdown and full score reports
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-actions github-actions Bot added the 🌟 core Changes to Starlight’s main package label May 31, 2026
@astrobot-houston

Copy link
Copy Markdown
Contributor

Hello! Thank you for opening your first PR to Starlight! ✨

Here’s what will happen next:

  1. Our GitHub bots will run to check your changes.
    If they spot any issues you will see some error messages on this PR.
    Don’t hesitate to ask any questions if you’re not sure what these mean!

  2. In a few minutes, you’ll be able to see a preview of your changes on Netlify 🤩

  3. One or more of our maintainers will take a look and may ask you to make changes.
    We try to be responsive, but don’t worry if this takes a few days.

@delucis delucis 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.

Thanks for the PR @miichom! And apologies for the slow response — I knew this would be a tricky one and needed time to play around with it to be sure the fix would work.

I’ve pushed up some tests for schema extension — previously we only touched that logic very minimally in an unrelated test. I included one for your case of extending a z.enum() property, which now passes on this branch (running that test against main fails, as you reported in #3935). However, the second part of this is type testing: this helps us ensure that the types inferred from an extended docs schema are correct.

Your changes seem to introduce an issue with the types when extending an object, e.g.

docsSchema({
	extend: z.object({ sidebar: z.object({ custom: z.number() }) }),
})

The default properties of the sidebar field are missing, with only the custom extension present. AFAICT this issue only impacts the types — you can see that in the failing test in CI. At runtime, all properties are parsed by Zod as expected.

Would you be able to take a look?

@HiDeoo

HiDeoo commented Jul 3, 2026

Copy link
Copy Markdown
Member

Your changes seem to introduce an issue with the types when extending an object, e.g.

Just to make sure, is only the type check an issue? Is this snapshot correct here and the result with sidebar default properties being dropped is expected?

@delucis

delucis commented Jul 3, 2026

Copy link
Copy Markdown
Member

Is this snapshot correct here and the result with sidebar default properties being dropped is expected?

Oh damn, you’re 100% correct. I thought I remembered checking this and seeing that everything expected was there, but nope, they’re dropped. So, yes, the issue is present for both runtime values AND types.

I updated that snapshot to reflect the runtime breakage too.

Comment thread packages/starlight/__tests__/basics/schema.test.ts
Comment thread packages/starlight/__tests__/basics/schema.test-d.ts
@HiDeoo

HiDeoo commented Jul 4, 2026

Copy link
Copy Markdown
Member

Played around with a potential fix in #4040.

I did that in a different PR to not mess around with the current PR where I cherry-picked all the commits from this PR in a single commit and added what I worked on top of it (also added 1 test and tweaked an existing one).

This is a draft as I'm not yet sure if this is the right approach or there are better ones, and I did not yet review it properly as I was so head down in the code that I did not yet take a step back. It's also overly commented as I was sometime getting lost when implementing it and I did not clean anything up yet.

If we ever decide to move forward with this, we should cherry pick only the relevant commits from #4040 in this PR so author credit is preserved and the history is cleaner.

@HiDeoo HiDeoo mentioned this pull request Jul 4, 2026
@miichom

miichom commented Jul 4, 2026

Copy link
Copy Markdown
Author

It's also overly commented as I was sometime getting lost when implementing it and I did not clean anything up yet.

I personally think for now keeping it commented is a good idea, I will have a proper look when I get home.

@miichom

miichom commented Jul 17, 2026

Copy link
Copy Markdown
Author

Okay, it took sometime but I preferred @HiDeoo schema merging over my one - I did remove the comments, but I am happy to re-add them if it helps others in the future. I added another test as the icon must start with <svg in the schema and would error without these PR changes.

@delucis delucis 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.

Thanks for your patience on this one @miichom!

I think we’re happy to accept this change with HiDeoo’s implementation. I wonder if it would be good to move most of this new code into a file under packages/starlight/utils/ to help keep the main schema.ts file focused on its primary purpose of declaring the schema. What do you think?

@miichom

miichom commented Jul 21, 2026

Copy link
Copy Markdown
Author

I think that is a great idea!

@delucis delucis 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.

Thanks @miichom — looking good! Just left a couple of last small notes.

Comment thread .changeset/all-badgers-grin.md Outdated
Comment thread packages/starlight/utils/zodDeepMerge.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🌟 core Changes to Starlight’s main package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

**hero.actions.1.variant**: Invalid option: expected one of "primary"|"secondary"|"minimal"

4 participants