Skip to content

fix(cli): Check existing conditions for specified options#771

Merged
manuel3108 merged 1 commit into
sveltejs:mainfrom
GrygrFlzr:fix-check-conditions
Nov 7, 2025
Merged

fix(cli): Check existing conditions for specified options#771
manuel3108 merged 1 commit into
sveltejs:mainfrom
GrygrFlzr:fix-check-conditions

Conversation

@GrygrFlzr
Copy link
Copy Markdown
Member

Closes #770 by making sure we actually apply conditions if they exist

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Nov 7, 2025

🦋 Changeset detected

Latest commit: bd3d39a

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

This PR includes changesets to release 1 package
Name Type
sv 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

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Nov 7, 2025

Open in StackBlitz

npx https://pkg.pr.new/sveltejs/cli/sv@771
npx https://pkg.pr.new/sveltejs/cli/svelte-migrate@771

commit: bd3d39a

@GrygrFlzr GrygrFlzr force-pushed the fix-check-conditions branch from 0d493fd to bd3d39a Compare November 7, 2025 04:40
specifiedOptions.map((option) => option.split(':', 2))
);
for (const option of specifiedOptions) {
let [optionId, optionValue] = option.split(':', 2);
Copy link
Copy Markdown
Member Author

@GrygrFlzr GrygrFlzr Nov 7, 2025

Choose a reason for hiding this comment

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

We could instead do

for (const option of Object.entries(specifiedOptionsObject)) {
    let [optionId, optionValue] = option;

But I'm not sure that's clearer intent.

Copy link
Copy Markdown
Member

@manuel3108 manuel3108 left a comment

Choose a reason for hiding this comment

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

This is perfect, thank you!

@manuel3108 manuel3108 merged commit 81c7351 into sveltejs:main Nov 7, 2025
7 of 8 checks passed
@github-actions github-actions Bot mentioned this pull request Nov 7, 2025
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.

runAddCommand doesn't actually test for conditions

2 participants