Skip to content

chore: remove __SVELTEKIT_DEV__ global#14308

Merged
teemingc merged 8 commits into
mainfrom
chore-use-esm-env-dev
Aug 26, 2025
Merged

chore: remove __SVELTEKIT_DEV__ global#14308
teemingc merged 8 commits into
mainfrom
chore-use-esm-env-dev

Conversation

@teemingc

@teemingc teemingc commented Aug 25, 2025

Copy link
Copy Markdown
Member

This PR addresses one of the TODO comments by consolidating our dev environment checks to use esm-env instead of relying on the (probably legacy) __SVELTEKIT_DEV__ global variable. It also adjusts some unit tests to run in production mode from the Vitest CLI where previously we just manipulated the global variable (but as far as I'm aware that's not possible to do inline in a test in Vitest to get esm-env to behave similarly)

Not sure if this is breaking or not but if it is let me know so we can adjust the changeset or delay the merging of this PR.


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

changeset-bot Bot commented Aug 25, 2025

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 0582e98

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

@svelte-docs-bot

Copy link
Copy Markdown

Comment thread .changeset/true-moles-tap.md Outdated
@Rich-Harris

Copy link
Copy Markdown
Member

This seems reasonable but the test failures look legit. Haven't investigated them though

@teemingc

Copy link
Copy Markdown
Member Author

Gah it was some lint error for not importing node's process explicitly. A shame that the eslint extension doesn't report it earlier.

test('injects relative service worker', () => {
const content = read('index.html');
expect(content).toMatch("navigator.serviceWorker.register('./service-worker.js')");
expect(content).toMatch("navigator.serviceWorker.register('./service-worker.js'");

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Removed the parentheses at the end so that this test passes whether it's ('./service-worker.js', { type: 'module' }) in dev or ('./service-worker.js') in prod

@teemingc teemingc requested a review from Rich-Harris August 26, 2025 06:32
@teemingc teemingc merged commit 8cf71f0 into main Aug 26, 2025
22 checks passed
@teemingc teemingc deleted the chore-use-esm-env-dev branch August 26, 2025 14:39
@github-actions github-actions Bot mentioned this pull request Aug 26, 2025
Copilot AI pushed a commit to Stadly/kit that referenced this pull request Mar 6, 2026
* remove __SVELTEKIT_DEV__ global

* changeset

* group third party import statements

* separate prod only tests

* format

* Apply suggestion from @eltigerchino

* explicitly import process

* adjust test to work in dev
teemingc added a commit that referenced this pull request May 15, 2026
fixes #15046
fixes #15632


This PR fixes a regression from
#14308 where `esm-env` replaced the
global constant for development checks by reverting to the global
constant where we want the code to only run during `vite dev` (rather
than following the `NODE_ENV` value). It also consolidates other places
that were incorrectly using `esm-env`. Every other place that uses `DEV`
is meant to fire warnings or avoid using the cache and that should still
happen if the user did `NODE_ENV=development vite build`

---

### 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.
- [ ] 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.
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.

2 participants