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 for ekhaled/svelte-hot-loader#10 #44

Merged
merged 1 commit into from
Mar 14, 2018

Conversation

ekhaled
Copy link
Contributor

@ekhaled ekhaled commented Mar 8, 2018

See ekhaled/svelte-hot-loader#10 for a bit of background.

The problem was...
While components were wrapped up in a proxy on initial load, after a hot reload only the instances were re-rendered. The wrapped component was never exported.

This works fine for nested components. But, for top-level components, it caused webpack to update its reference of the component to the unwrapped component.

When the top-level component was being mounted later on, using new Component, the unwrapped component was mounted.
The unwrapped component has no idea how to handle hot reloads... therefore it wasn't responding to HMR updates.

This PR fixes the immediate problem.
But it has opened my eyes to problems that I might have to tackle in the future.

For example:
When a new component comes in via hot reload, it's proxy needs to update it's pointers to the raw component's static properties, and static methods.
Right now these pointers/references are directed to the original.

For now, I think it's ok to expect people to do a full reload if they modify static properties and/or methods of a component.

} else {
// hot update
reload(${id}, proxyComponent);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line was causing the problem

@ekhaled
Copy link
Contributor Author

ekhaled commented Mar 9, 2018

@Rich-Harris Let me know if you have any questions regarding this PR.

It fixes HMR being lost, especially for Sapper (or anything that manually initialises components using new Component({})).

AFAICT this is ready to merge.

@Rich-Harris Rich-Harris merged commit 6b0dd9e into sveltejs:master Mar 14, 2018
@Rich-Harris
Copy link
Member

thank you! released 2.5.1

@ekhaled ekhaled deleted the dev-helper-gh-10 branch March 20, 2018 15:47
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