Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: store a persistent HMR wrapper per component #12537

Closed
wants to merge 3 commits into from

Conversation

dummdidumm
Copy link
Member

The previous HMR logic created a new wrapper and source per HMR update, which meant things would either get wrapped more and more (prior to #12454) or would not get updated after the first update because the reference to the original would get lost (after #12454).

This fixes that by creating a registry within the HMR wrappers by filename are stored, retrieved and its signals updated. That way nothing gets lost, and nothing gets wrapped more than needed.

Fixes #12506

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
  • Prefix your PR title with feat:, fix:, chore:, or docs:.
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests and linting

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

The previous HMR logic created a new wrapper and source per HMR update, which meant things would either get wrapped more and more (prior to #12454) or would not get updated after the first update because the reference to the original would get lost (after #12454).

This fixes that by creating a registry within the HMR wrappers by filename are stored, retrieved and its signals updated. That way nothing gets lost, and nothing gets wrapped more than needed.

Fixes #12506
Copy link

changeset-bot bot commented Jul 22, 2024

🦋 Changeset detected

Latest commit: a8c84f9

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

This PR includes changesets to release 1 package
Name Type
svelte 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

@dummdidumm
Copy link
Member Author

The runtime tests are failing because we're using the filename to check for pre-existing components, and the registry is preserved between tests and all tests have a main.svelte, and the rootDir option set in such a way that the resulting filenames are the same.

I don't know if that means we gotta adjust our registry key, or if we "just" need to have some HMR registry reset mechanism after each test

@Rich-Harris
Copy link
Member

nearly broke my brain but i think i figured it out #12547

@Rich-Harris
Copy link
Member

merged #12547, so will close this

@dummdidumm dummdidumm deleted the hmr-update-fix branch July 22, 2024 22:51
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.

Svelte 5: HMR only updates once
2 participants