Skip to content

Svelte: Resolve existing Knip findings - #36768

Merged
valentinpalkovic merged 2 commits into
nextfrom
svelte-knip-cleanup
Oct 14, 2026
Merged

valentinpalkovic merged 2 commits into
nextfrom
svelte-knip-cleanup

Conversation

@valentinpalkovic

Copy link
Copy Markdown
Contributor

What I did

Resolve existing Knip findings in the Svelte renderer, Svelte Vite framework, and SvelteKit framework without changing package exports or runtime behavior.

Summary

+ Derive Svelte and SvelteKit entry points from their build configs
+ Read the renderer Nx check target and declare ESTree type dependencies
- Unused internal helpers, test extractor, and redundant dependencies

The compiled Svelte fixture remains available to the tests that read it. It is excluded from source analysis. The Nx executor exception applies only to the root-owned tool. Public StoryContext, preset hooks, and SvelteKit mock exports remain intact.

Evidence

Knip scope Before After
Three Svelte packages 43 0
Repository 1,071 1,028

The baseline is 1921070f11c4a06c83b6a35c45d5e28dbca36b44. All 43 removed findings are in these three packages. There are no new findings anywhere. Repository-wide Knip still exits with the unrelated baseline findings.

Affected package compilation, all three type checks, focused lint, package manifest lint, and formatting pass. The Svelte package suites pass 225 tests across 45 files. Nx Cloud returned an access warning; local builds and checks completed successfully.

Merge Danger

Two-way door. The cleanup can be reverted. The affected packages retain their supported entries and behavior.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

Caution

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

  1. Install and compile from the repository root with yarn install and yarn nx run-many -t compile.
  2. Run yarn knip --workspace renderers/svelte --workspace frameworks/svelte-vite --workspace frameworks/sveltekit --reporter json. Expect {"issues":[]} and exit code 0. The same command on the baseline reports 43 findings.
  3. Run yarn knip --reporter json on baseline and head. Expect the existing repository findings to drop by exactly 43, with no additions outside the Svelte packages.
  4. Run yarn nx run-many -t check --projects=svelte,svelte-vite,sveltekit. Expect all three checks to pass.
  5. Run yarn test --project @storybook/svelte --project @storybook/svelte-vite --project @storybook/sveltekit. Expect all 225 tests to pass, including compiled Svelte CSF fixtures and portable stories.
  6. Inspect the three package export maps against the baseline. Expect no changes to exports, including the preset hooks and SvelteKit mock modules.

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts

  • Declare whether manual QA will be needed for this PR during the next release, through qa:needed or qa:skip

  • Make sure this PR contains one of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentation only changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

🦋 Canary Release - 🚫 Not run

This PR does not have a canary release associated.

In-repo PRs: add the ci:canary label. Later pushes republish while the label remains.

Fork PRs: the label does nothing (a later push must not auto-publish). A maintainer publishes from this repository with Run workflow and the pr input. branch and sha are optional; if more than one is set, they must be the same commit. The fork author does not need to do anything.

@valentinpalkovic valentinpalkovic added cleanup Minor cleanup style change that won't show up in release changelog ci:normal Run our default set of CI jobs (choose this for most PRs). qa:skip Pull Requests that do not need any QA. (e.g. documentation) labels Oct 13, 2026 — with Paperclip for GitHub
@valentinpalkovic
valentinpalkovic marked this pull request as ready for review October 13, 2026 19:05
@valentinpalkovic
valentinpalkovic requested review from a team and winkerVSbecks and removed request for a team October 13, 2026 19:05
@coderabbitai

coderabbitai Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: bab29b96-81e9-4115-871f-4d7071319b31
📥 Commits

Reviewing files that changed from the base of the PR and between 1921070 and 09ab61f.

⛔ Files ignored due to path filters: 1
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing: 11
  • code/frameworks/svelte-vite/package.json
  • code/frameworks/svelte-vite/src/plugins/generateDocgen.ts
  • code/renderers/svelte/package.json
  • code/renderers/svelte/src/__test__/composeStories/portable-stories.test.ts
  • code/renderers/svelte/src/svelte-csf/__tests__/extractor.ts
  • code/renderers/svelte/src/svelte-csf/compiler/post-transform/shared/parameters.ts
  • code/renderers/svelte/src/svelte-csf/parser/ast.ts
  • code/renderers/svelte/src/svelte-csf/utils/identifier-utils.ts
  • code/renderers/svelte/src/svelte-csf/utils/import-source.ts
  • code/renderers/svelte/src/types.ts
  • scripts/knip.config.ts
💤 Files with no reviewable changes: 3
  • code/renderers/svelte/src/svelte-csf/tests/extractor.ts
  • code/renderers/svelte/src/svelte-csf/utils/identifier-utils.ts
  • code/renderers/svelte/src/svelte-csf/parser/ast.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Walkthrough

Svelte package development dependencies and Knip workspace entries changed. Several Svelte types and helper functions are no longer exported, and an AST extraction test helper was deleted.

Changes

Svelte package and API maintenance

Layer / File(s) Summary
Package tooling and workspace entries
code/frameworks/svelte-vite/package.json, code/renderers/svelte/package.json, scripts/knip.config.ts
The package manifests add @types/estree and remove the listed development dependencies. Knip now uses Svelte and SvelteKit build entries and defines workspaces for both packages.
Svelte module and test helper cleanup
code/frameworks/svelte-vite/src/plugins/generateDocgen.ts, code/renderers/svelte/src/__test__/composeStories/portable-stories.test.ts, code/renderers/svelte/src/svelte-csf/*, code/renderers/svelte/src/types.ts
PropInfo and several Svelte renderer helpers and types are no longer exported. The AST extraction helper file was deleted. The portable stories test now imports expectTypeOf from Vitest.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 09ab6

No actionable regression is established by the reviewed changes. The PR is mergeable after normal checks.

✨ Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@winkerVSbecks

Copy link
Copy Markdown
Contributor

@valentinpalkovic I'm not the right person to review this.

@valentinpalkovic valentinpalkovic self-assigned this Oct 13, 2026
@valentinpalkovic
valentinpalkovic requested review from a team and removed request for winkerVSbecks October 13, 2026 19:21
@valentinpalkovic
valentinpalkovic merged commit 40680d2 into next Oct 14, 2026
161 of 167 checks passed
@valentinpalkovic
valentinpalkovic deleted the svelte-knip-cleanup branch October 14, 2026 08:12
ghengeveld added a commit to storybookjs/sandboxes that referenced this pull request Oct 14, 2026
Check the diff here: storybookjs/storybook@8af3851...50a8282

List of included PRs since previous version:
- storybookjs/storybook#36301 (sharadvc/cursor/fix-mcp-connect-agents-link-6fef)
- storybookjs/storybook#35863 (SnowingFox/fix/35862-server-channel-error-handler)
- storybookjs/storybook#36688 (ghengeveld/vitest-36221-leftovers)
- storybookjs/storybook#36746 (fix/bbc-local-story-manifest-snippets)
- storybookjs/storybook#36588 (fix/nextjs-router-mock-bfcache)
- storybookjs/storybook#36391 (agent/sto-492-eslint-cjs)
- storybookjs/storybook#36751 (agent/sto-280-prerelease-version-alignment)
- storybookjs/storybook#36768 (svelte-knip-cleanup)
- storybookjs/storybook#35834 (FrancoKaddour/fix/docgen-typescript7-missing-compiler-api)
- storybookjs/storybook#36770 (bryce-easygo/fix/svelte-csf-null-arg-snippet)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:normal Run our default set of CI jobs (choose this for most PRs). cleanup Minor cleanup style change that won't show up in release changelog qa:skip Pull Requests that do not need any QA. (e.g. documentation)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants