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: support new location for content config #12475

Merged
merged 7 commits into from
Nov 21, 2024
Merged

Conversation

ascorbic
Copy link
Contributor

@ascorbic ascorbic commented Nov 19, 2024

Changes

Adds support for content config files in src/content.config.*. This means there no longer needs to be a src/content folder.

Fixes PLT-2649

Testing

Config files moved for most fixtures, though with some legacy collections kept in the old location to ensure test coverage.

Docs

Docs will need updating with the new location, as eventually will templates etc.

Copy link

changeset-bot bot commented Nov 19, 2024

🦋 Changeset detected

Latest commit: c70d181

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: example Related to an example package (scope) pkg: astro Related to the core `astro` package (scope) docs pr labels Nov 19, 2024
@ascorbic ascorbic marked this pull request as ready for review November 19, 2024 14:11
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.

The description of the PR is a bit vague. Here's some questions/concerns regarding the changes:

  • we are still supporting the old content collections in it, should we disallow this?
  • what's the plan of the old file?
  • is it possible to use the same logic we use to load known files in Astor?

.changeset/thirty-clocks-jump.md Outdated Show resolved Hide resolved

Changes the default content config location from `src/content/config.*` to `src/content.config.*`.

The previous location is still supported, but the new location is now preferred.
Copy link
Member

Choose a reason for hiding this comment

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

What's the plan for the previous one? Is it deprecated?

Copy link
Member

Choose a reason for hiding this comment

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

@ascorbic should update the changeset, and mention that the new config doesn't old content collections? (At least, that's what I understood from one of your comments).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, updated to say that the old location is still supported, and is required for legacy mode.

packages/astro/src/content/runtime.ts Outdated Show resolved Hide resolved
packages/astro/src/content/utils.ts Outdated Show resolved Hide resolved
packages/astro/src/core/errors/errors-data.ts Outdated Show resolved Hide resolved
packages/astro/test/astro-sync.test.js Outdated Show resolved Hide resolved
@github-actions github-actions bot added the semver: minor Change triggers a `minor` release label Nov 19, 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.

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.

Few things that I couldn't understand:

  • is content/config.ts considered deprecated?
  • is there a plan for the "old" configuration file? If so, which one is it?

Code wise it looks good though


Changes the default content config location from `src/content/config.*` to `src/content.config.*`.

The previous location is still supported, but the new location is now preferred.
Copy link
Member

Choose a reason for hiding this comment

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

@ascorbic should update the changeset, and mention that the new config doesn't old content collections? (At least, that's what I understood from one of your comments).

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.

@ascorbic ascorbic merged commit 3f02d5f into next Nov 21, 2024
14 checks passed
@ascorbic ascorbic deleted the move-content-config branch November 21, 2024 10:45
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) pkg: example Related to an example package (scope) semver: minor Change triggers a `minor` release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants