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

💽 Add site options to each page #1536

Merged
merged 2 commits into from
Sep 19, 2024
Merged

💽 Add site options to each page #1536

merged 2 commits into from
Sep 19, 2024

Conversation

rowanc1
Copy link
Collaborator

@rowanc1 rowanc1 commented Sep 14, 2024

This enables per-page options for the site.

Picked up in the theme in jupyter-book/myst-theme#472

Copy link

changeset-bot bot commented Sep 14, 2024

🦋 Changeset detected

Latest commit: 62a5836

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

This PR includes changesets to release 6 packages
Name Type
myst-frontmatter Patch
myst-cli Patch
myst-common Patch
myst-config Patch
myst-spec-ext Patch
mystmd 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

Copy link
Collaborator

@fwkoch fwkoch left a comment

Choose a reason for hiding this comment

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

Validating options against the site template looks good and makes sense.

My only concern, though, is introducing site options, alongside the existing options field. In my mind, options covers all template-specific options including both site and static export templates... I'm worried about confusion around putting an option in the wrong options location.

packages/myst-cli/src/frontmatter.ts Show resolved Hide resolved
if (defined(value.site)) {
// These are validated later based on the siteTemplate
// At this point, they just need to be an object
output.site = validateObject(value.site, incrementOptions('site', opts));
Copy link
Collaborator

Choose a reason for hiding this comment

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

How does this overlap with options, which already exists per-page?

@fwkoch
Copy link
Collaborator

fwkoch commented Sep 19, 2024

Chatting some with @rowanc1 - we will keep the new site key, as a place to override site options at a page level. This is somewhat similar to how we treat other exports, where in the export definition, you can define other options and override fields. (There is some discussion of sites vs. exports here: #1103) Also, once we tackle removing the project key in myst.yml (#1308), page frontmatter with site alongside other keys will start looking very similar to myst.yml content with site alongside other keys.

As for options - these can remain, as a place to define any arbitrary key/value pair. The site/export templates should have access to these options, but they are less strictly validated than options defined directly on site.

Also note, I think this PR addresses: #1450

@fwkoch fwkoch merged commit 760e411 into main Sep 19, 2024
7 checks passed
@fwkoch fwkoch deleted the theme/opts branch September 19, 2024 17:46
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.

2 participants