Skip to content

feat: ignore *.{test,spec}.{js,ts} and similar files - #13009

Closed
rChaoz wants to merge 5 commits into
sveltejs:mainfrom
rChaoz:feat/test-extensions
Closed

feat: ignore *.{test,spec}.{js,ts} and similar files#13009
rChaoz wants to merge 5 commits into
sveltejs:mainfrom
rChaoz:feat/test-extensions

Conversation

@rChaoz

@rChaoz rChaoz commented Nov 15, 2024

Copy link
Copy Markdown
Contributor

Closes #13008, please see issue for more details


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.

@rChaoz
rChaoz force-pushed the feat/test-extensions branch from 6c43108 to 6e0f0eb Compare November 15, 2024 16:55
@changeset-bot

changeset-bot Bot commented Nov 15, 2024

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: da632f3

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

changeset-bot Bot commented Nov 15, 2024

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 6e0f0eb

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

@Rich-Harris

Copy link
Copy Markdown
Member

preview: https://svelte-dev-git-preview-kit-13009-svelte.vercel.app/

this is an automated message

@rChaoz rChaoz changed the title Ignore +server.test.ts and similar files feat: ignore \*.{test,spec}.{js,ts} and similar files Nov 15, 2024
@rChaoz rChaoz changed the title feat: ignore \*.{test,spec}.{js,ts} and similar files feat: ignore *.{test,spec}.{js,ts} and similar files Nov 15, 2024
@teemingc teemingc added the feature / enhancement New feature or request label Nov 18, 2024
@dummdidumm

Copy link
Copy Markdown
Member

I don't think we need extra config for this - which could also be abused to hide files from the router which it should otherwise not (like a backdoor to customize your routing scheme in weird ways). .test/.spec.{js.ts} will suffice

@rChaoz

rChaoz commented Jan 23, 2025

Copy link
Copy Markdown
Contributor Author

I don't agree with that. First, because you may have different/multiple extensions, or a combination. It's not rare to see:

  • x.test.svelte.js / x.svelte.test.js
  • other extensions like .stories, for example used with +page.stories.svelte
  • separation between unit/int tests, for example x.test.js and x.int.test.js/x.int.js

Also, I don't believe something shouldn't be added just because there is a chance it can be misused.

@rChaoz
rChaoz force-pushed the feat/test-extensions branch from 6e0f0eb to b5d60b8 Compare January 23, 2025 19:53
@rChaoz
rChaoz force-pushed the feat/test-extensions branch from a1c4974 to d4298f8 Compare January 25, 2025 00:01
@Rich-Harris

Copy link
Copy Markdown
Member

which could also be abused to hide files from the router

Maybe that's a feature? Because that's what this is actually doing — testExtensions is a misnomer, given that it doesn't magically wire up your tests or whatever. It just hides stuff from the router.

So it could be something like this instead:

// svelte.config.js
export default {
  kit: {
    router: {
      ignore: /\.(test|spec|stories)\./
    }
  }
};

I think we'd probably only want to apply it to the basename, and it should error if a valid route file (i.e. +page.svelte etc) was ignored, so that people didn't abuse it.

Thoughts? Should the regex in that example be the default?

@Rich-Harris Rich-Harris added the needs-decision Not sure if we want to do this yet, also design work needed label Aug 20, 2025
@svelte-docs-bot

Copy link
Copy Markdown

@Rich-Harris Rich-Harris added this to the 3.0 milestone Aug 5, 2026
@teemingc teemingc added needs-ecosystem-ci This PR needs to have an ecosystem CI run started prior to its last commit before merging. and removed needs-ecosystem-ci This PR needs to have an ecosystem CI run started prior to its last commit before merging. labels Aug 5, 2026
@Rich-Harris

Copy link
Copy Markdown
Member

We decided to go with a non-config approach, so closing this in favour of #16715 — thank you!

Rich-Harris added a commit that referenced this pull request Aug 10, 2026
…16715)

Simpler alternative to #13009. We decided it's probably fine — and
presents less risk of forcing breaking changes on us in future — to just
hardcode this list. If that turns out to be the wrong decision we can
easily add configuration like `router.ignore` as proposed in
#13009 (comment), but
for now this seems like the easier option.

Closes #13008 

---

### 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature / enhancement New feature or request needs-decision Not sure if we want to do this yet, also design work needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ignore +server.test.ts and similar files

4 participants