Skip to content

feat(cli): nextSteps handle falsy values - #1215

Open
sacrosanctic wants to merge 7 commits into
sveltejs:mainfrom
sacrosanctic:next-steps-handle-false
Open

feat(cli): nextSteps handle falsy values#1215
sacrosanctic wants to merge 7 commits into
sveltejs:mainfrom
sacrosanctic:next-steps-handle-false

Conversation

@sacrosanctic

Copy link
Copy Markdown
Contributor

Closes #

Description

  • QoL change, falsy values are nicer to work with

Checklist

  • Update snapshots (if applicable)
  • Add a changeset (if applicable)
  • Allow maintainers to edit this PR
  • I care about what I'm doing, no matter the tool I use (Notepad, Sublime, VSCode, AI...)

@pkg-svelte-dev

pkg-svelte-dev Bot commented Aug 1, 2026

Copy link
Copy Markdown

Install the latest version of sv from ab80168:

pnpm add https://pkg.svelte.dev/sv/c/ab80168f98b54e538c1ff15a498ed1e92a156931

Open in pkg.svelte.dev: https://pkg.svelte.dev/repos/cli/pr/1215

Note

This PR is from a fork. A maintainer must approve approve each commit before it can be built and installed.

@changeset-bot

changeset-bot Bot commented Aug 1, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: ab80168

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@jycouet

jycouet commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

falsy values are nicer to work with

Yes?! I find them a bit more cryptic! I don't have a huge opinion on this. Maybe others wanna have a look?

I'll have to see why svelte.dev is not happy for cli, that's annoying.

@AdrianGonz97 AdrianGonz97 left a comment

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.

Yea, I agree with @jycouet on this one. I don't think allowing falsy values and filtering them out is a great improvement on clarity.

},

nextSteps: ({ options, packageManager, cwd, dependencyVersion }) => {
const pm = (command: Parameters<typeof resolveCommandArray>[1], args: string[]) =>

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.

this helper is a nice addition though. we could keep that

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Online here ? Or even higher ? To be used in other spots ? 👀

@GauBen

GauBen commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Yes?! I find them a bit more cryptic! I don't have a huge opinion on this. Maybe others wanna have a look?

I don't have a strong opinion on this, but

nextSteps: ({ isKit, packageManager }) =>
  [
    isKit && `Run ${color.command(resolveCommandArray(packageManager, "run", ["dev"]))} and consult ${color.website("http://localhost:5173/demo/markdown")} to see the result`,
    `Read the documentation at ${color.website("https://github.com/ota-meshi/vite-plugin-svelte-md")}`,
  ],

is a bit nicer than

nextSteps: ({ isKit, packageManager }) =>
  [
    `Run ${color.command(resolveCommandArray(packageManager, "run", ["dev"]))} and consult ${color.website("http://localhost:5173/demo/markdown")} to see the result`,
    `Read the documentation at ${color.website("https://github.com/ota-meshi/vite-plugin-svelte-md")}`,
  ].slice(isKit ? 0 : 1),

(From ota-meshi/vite-plugin-svelte-md#157)

@AdrianGonz97

AdrianGonz97 commented Aug 3, 2026

Copy link
Copy Markdown
Member

I mean, if people really want it, nothing stops them from just filtering out the falsy values themselves:

nextSteps: ({ isKit, packageManager }) =>
  [
    isKit && "...stuff 1",
	isKit && "...more stuff 2",
	isKit && packageManager === "pnpm" && "...even more stuff 3",
    "...always added stuff 4",
  ].filter(Boolean),

@sacrosanctic

Copy link
Copy Markdown
Contributor Author

@AdrianGonz97 typescript does not resolve that correctly. microsoft/TypeScript#16655

You'd need to do this

nextSteps: ({ isKit, packageManager }) => {
  const notNullable = (x:any):x is NonNullable<typeof x> => Boolean(x)

  return [
    isKit && "...stuff 1",
    isKit && "...more stuff 2",
    isKit && packageManager === "pnpm" && "...even more stuff 3",
    "...always added stuff 4",
  ].filter(notNullable)
}

@AdrianGonz97

AdrianGonz97 commented Aug 4, 2026

Copy link
Copy Markdown
Member

right, a bit more verbose to satisfy typescript then:

  nextSteps: ({ isKit, packageManager }) => [
      isKit && "...stuff 1",
      isKit && "...more stuff 2",
      isKit && packageManager === "pnpm" && "...even more stuff 3",
      "...always added stuff 4",
  ].filter((line): line is string => !!line),

annoying but still doable

@jycouet

jycouet commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

I'm such an old school guy that I like if & arr.push('Yop') ? haha

@sacrosanctic

Copy link
Copy Markdown
Contributor Author

This change has way more push back then I had anticipated. I thought it would be a slam dunk.

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.

4 participants