Skip to content

fix: sync override settings#14782

Merged
florian-lefebvre merged 4 commits into
mainfrom
fix/sync-override-settings
Nov 25, 2025
Merged

fix: sync override settings#14782
florian-lefebvre merged 4 commits into
mainfrom
fix/sync-override-settings

Conversation

@florian-lefebvre

@florian-lefebvre florian-lefebvre commented Nov 17, 2025

Copy link
Copy Markdown
Member

Changes

  • The settings are shared between several vite processes because of sync
  • This caused some code to rerun when it shouldn't
  • This PR overrides some settings to ensure sync does not mess with them

Testing

Should pass

Docs

Changeset. Technically not required but I prefer to play it safe

@florian-lefebvre florian-lefebvre self-assigned this Nov 17, 2025
@changeset-bot

changeset-bot Bot commented Nov 17, 2025

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 1ba4867

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 Nov 17, 2025
@github-actions

github-actions Bot commented Nov 17, 2025

Copy link
Copy Markdown
Contributor

📝 Changeset Validation Results

Changeset validation failed

Issues Found:

.changeset/tender-wasps-shop.md

Issue with: 'Improves syncing'

❌ The description does not adhere to the required verb tense and structure.

💡 Update to something like: Improves syncing functionality to ensure data consistency.


📖 See Astro's changeset guide for details.

@codspeed-hq

codspeed-hq Bot commented Nov 17, 2025

Copy link
Copy Markdown

CodSpeed Performance Report

Merging #14782 will not alter performance

Comparing fix/sync-override-settings (1ba4867) with main (8cf3f05)1

Summary

✅ 6 untouched

Footnotes

  1. No successful run was found on main (ad3265d) during the generation of this report, so 8cf3f05 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

Comment thread packages/astro/src/core/sync/index.ts
@florian-lefebvre florian-lefebvre marked this pull request as ready for review November 18, 2025 09:16
Comment thread packages/astro/src/core/sync/index.ts
Comment thread packages/astro/src/core/sync/index.ts
Comment thread packages/astro/src/core/sync/index.ts
Comment thread packages/astro/src/core/sync/index.ts
settings: {
...settings,
// Prevent mutation by vite plugins during sync
buildOutput: undefined,

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.

Do you think we can achieve this using TypeScript? I know it has the type Readonly

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't think so in this context. Because we still need it to be reassigned at some point, so we can't just update AstroSettings['buildOutput']

@florian-lefebvre florian-lefebvre merged commit abed929 into main Nov 25, 2025
24 of 25 checks passed
@florian-lefebvre florian-lefebvre deleted the fix/sync-override-settings branch November 25, 2025 08:05
@astrobot-houston astrobot-houston mentioned this pull request Nov 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg: astro Related to the core `astro` package (scope)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants