Skip to content

Conversation

@florian-lefebvre
Copy link
Member

@florian-lefebvre florian-lefebvre commented Mar 21, 2025

Changes

  • In the config schema, refine and superRefine were only called when validating the user's config
  • With this PR, those validations are moved to another schema, called for the user config (as before) as well as at the end of astro:config:setup for each integration
  • Config related schemas are split in several files for clarity
  • Integrations hooks are refactored
  • Unhandled errors in integration hooks now log which integration and which hook is failing
  • I recommend you review the PR without whitespace changes

Testing

Updated, should pass

Docs

Changeset, no docs update needed I think

@florian-lefebvre florian-lefebvre self-assigned this Mar 21, 2025
@changeset-bot
Copy link

changeset-bot bot commented Mar 21, 2025

🦋 Changeset detected

Latest commit: ef18425

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 the pkg: astro Related to the core `astro` package (scope) label Mar 21, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Mar 21, 2025

CodSpeed Performance Report

Merging #13482 will not alter performance

Comparing feat/post-integrations-config-validation (ef18425) with main (b8645c1)

Summary

✅ 6 untouched benchmarks

@github-actions github-actions bot added the semver: minor Change triggers a `minor` release label Mar 21, 2025
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 March 21, 2025 12:06
@florian-lefebvre florian-lefebvre marked this pull request as ready for review March 26, 2025 12:32
@florian-lefebvre florian-lefebvre added this to the v5.6.0 milestone Mar 26, 2025
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 believe the new names are misleading. Calling the new schema refined isn't correct IMHO, unless I misunderstood its intent. Now that we have a schema that runs after the hooks, we can also run transformations, right? Without affecting downstream integrations.

If so, can we call the new schema post validation or something similar?

@florian-lefebvre
Copy link
Member Author

@ematipico it's called refined on purpose because it only performs a set of checks inside of .superRefine(), there are no transformations happening. I think if we want to be able to do transforms before being stored in settings, that will require another PR (because eg. types will have to added/renamed across a lot of places etc)

@ematipico
Copy link
Member

Ok, I thought the initial intent of the PR was to allow transformations after validation.

Copy link
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

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

Thanks for the ping @florian-lefebvre! Left some comments.

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.

Approving for docs!

@ematipico
Copy link
Member

@delucis, I am going to merge this PR. Please let us know if you have concerns.

@ematipico ematipico merged commit ff257df into main Apr 2, 2025
17 checks passed
@ematipico ematipico deleted the feat/post-integrations-config-validation branch April 2, 2025 10:09
@astrobot-houston astrobot-houston mentioned this pull request Apr 2, 2025
openscript pushed a commit to openscript/astro that referenced this pull request Sep 12, 2025
…o#13482)

* feat: move astro config validation after astro:config:setup

* feat: superRefine

* fix: test

* feat: update test

* feat: update test

* fix: test

* feat: feedback

* feat: improve logging

* feat: refactor

* fix: no spread

* feat: split schemas

* chore: changeset

* Update packages/astro/src/core/config/schemas/README.md

Co-authored-by: Emanuele Stoppa <[email protected]>

* Update packages/astro/src/integrations/hooks.ts

* Update packages/astro/src/core/config/schemas/README.md

* Update .changeset/easy-vans-laugh.md

* Update .changeset/easy-vans-laugh.md

* grammar nit in changeset

---------

Co-authored-by: Emanuele Stoppa <[email protected]>
Co-authored-by: Sarah Rainsberger <[email protected]>

Co-authored-by: ematipico <[email protected]>
Co-authored-by: delucis <[email protected]>
Co-authored-by: sarah11918 <[email protected]>
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