Skip to content

Conversation

jasonlyu123
Copy link
Member

@jasonlyu123 jasonlyu123 commented Sep 18, 2025

Alternative to the module cache handling of #2852. Still have to check why the "adding all missing imports" test failed in local.

Copy link

changeset-bot bot commented Sep 18, 2025

🦋 Changeset detected

Latest commit: 70f4373

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

This PR includes changesets to release 2 packages
Name Type
svelte-language-server Patch
svelte-check 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

@43081j
Copy link
Contributor

43081j commented Sep 18, 2025

this looks great

locally i can see a huge speed up in createSnapshot too (now <10ms self vs 600 before)

This makes the test do extra work after everything is done and creates weird race conditions, which I have already tried to fix many times
@jasonlyu123 jasonlyu123 marked this pull request as ready for review September 19, 2025 04:09
});

if (toRemoves.length) {
moduleCache.deleteByValues(Array.from(toRemoves));
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a reason we clone the array here? does deleteByValues mutate it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Eh. I forgot to remove it. toRemoves was originally a Set.

Copy link
Contributor

@43081j 43081j left a comment

Choose a reason for hiding this comment

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

all looks good to me 🙏

it runs a lot faster 😄

@jasonlyu123
Copy link
Member Author

For svelte-check and editor startup, the change is mainly skipping the cache invalidation check. getSnapshot and getSnapshotIfExist both get called because it's resolved through module resolution or the file is a project file. So we should already know that the file exists. The credit should still belong to @43081j for finding out that the module cache is immediately invalidated after it was created.

And for the cache invalidation check, it was scheduled to run before TypeScript builds the program. The check is done by checking the failed file path that TypeScript recorded during module resolution. So it should only run after the first load since we won't have the info before that.

Copy link
Member

@dummdidumm dummdidumm left a comment

Choose a reason for hiding this comment

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

Awesome!

@dummdidumm dummdidumm merged commit ec7be4b into sveltejs:master Sep 19, 2025
3 checks passed
@github-actions github-actions bot mentioned this pull request Sep 19, 2025
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.

3 participants