Skip to content

fix: consistently use bare import for internals#14506

Merged
dummdidumm merged 1 commit intomainfrom
bare-internal-import
Sep 22, 2025
Merged

fix: consistently use bare import for internals#14506
dummdidumm merged 1 commit intomainfrom
bare-internal-import

Conversation

@Rich-Harris
Copy link
Member

In #14447 I broke a cardinal rule — code in src/runtime should only ever import code from src/exports via the public bare imports made available via pkg.exports. Otherwise, there's a danger that a given module will be loaded through both Vite and Node, causing breakage.

I ignored that rule because I was trying to deal with some tricky resolution bug, and #14505 was my reward. Luckily I think the tricky resolution bug is solved by not having a nested pkg.exports map, and instead abusing pkg.imports to achieve the intended outcome.

This is one of those things that's extremely hard to test for from within the workspace, because of how Vite works. But I think this solution is good. Closes #14505.


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

  • 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
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • 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

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

@changeset-bot
Copy link

changeset-bot bot commented Sep 22, 2025

🦋 Changeset detected

Latest commit: 6467e0f

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 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

@Rich-Harris
Copy link
Member Author

(Tested the repro in #14505 with the pkg.pr.new version from this PR, and it does indeed appear to be fixed)

@dummdidumm dummdidumm merged commit cc316bc into main Sep 22, 2025
22 checks passed
@dummdidumm dummdidumm deleted the bare-internal-import branch September 22, 2025 18:11
@github-actions github-actions bot mentioned this pull request Sep 22, 2025
dummdidumm added a commit that referenced this pull request Sep 22, 2025
… src/runtime

So that bugs that e.g. #14506 had to fix don't happen again
@@ -1,6 +1,6 @@
import { base, assets, relative } from './internal/server.js';
Copy link
Member

Choose a reason for hiding this comment

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

What's the reason for this import not using the import map $apps/paths/internal/server instead? Does the rule only apply to internal paths using the export map?

dummdidumm added a commit that referenced this pull request Sep 23, 2025
… src/runtime (#14510)

So that bugs that e.g. #14506 had to fix don't happen again
Copilot AI pushed a commit to Stadly/kit that referenced this pull request Mar 6, 2026
Copilot AI pushed a commit to Stadly/kit that referenced this pull request Mar 6, 2026
… src/runtime (sveltejs#14510)

So that bugs that e.g. sveltejs#14506 had to fix don't happen again
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.

Cannot use getRequestEvent in kit 2.43.0

4 participants