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

feat(next): make defineConfig generic #12243

Merged
merged 21 commits into from
Oct 21, 2024
Merged

Conversation

florian-lefebvre
Copy link
Member

@florian-lefebvre florian-lefebvre commented Oct 16, 2024

Changes

  • Makes defineConfig generic (backward compatible). That means that some parts of the config will be constrained by others. For example, it will ensure that i18n.defaultLocale is one of i18n.locales at the type level
  • Reworks how astro/config works, by having one source of truth (from config.d.ts/config.mjs to src/config/entrypoint.ts being compiled)

Testing

Should still pass, I added types tests

Docs

I'll do a docs PR about i18n once approved

Copy link

changeset-bot bot commented Oct 16, 2024

🦋 Changeset detected

Latest commit: 4dad468

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

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

@github-actions github-actions bot added pkg: astro Related to the core `astro` package (scope) docs pr labels Oct 16, 2024
@github-actions github-actions bot added the semver: minor Change triggers a `minor` release label Oct 16, 2024
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This PR is blocked because it contains a minor changeset. A reviewer will merge this at the next release if approved.

@florian-lefebvre florian-lefebvre marked this pull request as ready for review October 16, 2024 14:56
Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

I have no idea what the new TS code is doing 😅 but I saw the demo, and I liked it!

packages/astro/src/config/entrypoint.ts Outdated Show resolved Hide resolved
packages/astro/src/config/entrypoint.ts Show resolved Hide resolved
packages/astro/src/types/public/config.ts Outdated Show resolved Hide resolved
Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Changeset looks good, but just checking that I don't see any specific error messages in this PR.

I'm fine with changing the order of locales and defaultLocale, but I'm not sure what if any docs changes would be required for this PR, since this doesn't change the fact that we do already say that the default locale must be in your list of supported locales. Happy for you to double check that in the docs repo though!

.changeset/forty-trains-notice.md Outdated Show resolved Hide resolved
@florian-lefebvre florian-lefebvre merged commit eb41d13 into next Oct 21, 2024
14 checks passed
@florian-lefebvre florian-lefebvre deleted the feat/generic-define-config branch October 21, 2024 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs pr pkg: astro Related to the core `astro` package (scope) semver: minor Change triggers a `minor` release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants