kit icon indicating copy to clipboard operation
kit copied to clipboard

feat: add option to recursively consider `server` dirs in $lib

Open rChaoz opened this issue 1 year ago • 3 comments

Closes #12732

This PR adds a new config option kit.files.nestedServerDirs that, when enabled, considers any modules inside any $lib/**/server/** directory a server module, rather than just $lib/server/**.


Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • [x] It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • [x] This message body should clearly illustrate what problems it solves.
  • [x] Ideally, include a test that fails without this PR but passes with it.

Tests

  • [x] Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • [x] If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

Edits

  • [x] Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.

rChaoz avatar Sep 28 '24 21:09 rChaoz

🦋 Changeset detected

Latest commit: 97466409192b46d726123065b606e74b37c59a9e

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

This PR includes changesets to release 1 package
Name Type
@sveltejs/kit Minor

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

changeset-bot[bot] avatar Sep 28 '24 21:09 changeset-bot[bot]

Instead of adding an option, can we instead make this the default? Would that be considered a breaking change?

teemingc avatar Oct 06 '24 15:10 teemingc

Instead of adding an option, can we instead make this the default? Would that be considered a breaking change?

Yes because if for some reason I'm importing a module on the client from /server/nested/module.ts it will fail with this

paoloricciuti avatar Oct 06 '24 15:10 paoloricciuti

Related #12480

dummdidumm avatar Jan 15 '25 15:01 dummdidumm

Should I bring this up-to-date? Or maybe can this be considered to become the default in v3?

rChaoz avatar Jan 15 '25 16:01 rChaoz

I think it would make sense for this to be the default in v3 — it's probably not worth adding a new option in the meantime, and since this PR has a lot of merge conflicts I'll go ahead and close it. I've given #12732 a 3.0 milestone to ensure it doesn't get left out. Thanks!

Rich-Harris avatar Aug 18 '25 20:08 Rich-Harris