breaking: nested server-only directories#15685
breaking: nested server-only directories#15685rChaoz wants to merge 4 commits intosveltejs:version-3from
Conversation
🦋 Changeset detectedLatest commit: 26bce18 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
rChaoz
force-pushed
the
feat/recursive-server-dirs
branch
from
2244f2d to
2f75514
Compare
rChaoz
force-pushed
the
feat/recursive-server-dirs
branch
from
2f75514 to
cd3c44f
Compare
rChaoz
changed the title
$lib server dirs$lib server dirs
| '@sveltejs/kit': major | ||
| --- | ||
|
|
||
| recursive $lib server dirs |
There was a problem hiding this comment.
| recursive $lib server dirs | |
| breaking: nested server-only directories |
|
|
||
| - adding `.server` to the filename, e.g. `secrets.server.js` | ||
| - placing them in `$lib/server`, e.g. `$lib/server/secrets.js` | ||
| - For single modules, add `.server` to the filename, e.g. `secrets.server.js`. This works for _any_ file in the project directory, including within `$lib` or `$routes`. |
There was a problem hiding this comment.
I don't think we automatically create a $routes alias, do we?
There was a problem hiding this comment.
Right, I've had one in my projects for so long I forgot it's not native
| const import_map = new Map(); | ||
| const server_only_pattern = /.*\.server\..+/; | ||
| // Matches any ID that has .server. in its filename | ||
| const server_only_module_pattern = /\.server\.[^/]+$/; |
There was a problem hiding this comment.
did we change this from what it was before on purpose?
There was a problem hiding this comment.
Yes, for consistency
| normalized.startsWith('$lib/server/') || | ||
| (is_internal && server_only_pattern.test(path.basename(id))); | ||
| (normalized.startsWith('$lib/') && server_only_directory_pattern.test(id)) || | ||
| (is_internal && server_only_module_pattern.test(id)); |
There was a problem hiding this comment.
Was there any reason to change this?
There was a problem hiding this comment.
Same reason as above. It also prevents false positives because the hook ID filters no longer match .server in directory names
There was a problem hiding this comment.
These tests are good, but we should probably add another two apps to build-errors for nested server directories, both dynamic and static imports. You should be able to just ctrl+c, ctrl+v one of the existing test apps in build-errors and modify from there. The tests are in env.spec.js
elliott-with-the-longest-name-on-github
changed the title
$lib server dirs2 participants
closes #12732
Instead of only considering
$lib/server/**as server-only modules, consider$lib/**/server/**instead, see issue above for reasoning.As per the conversation in #12733, adding this as defaults for v3 (breaking change).
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.Edits