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

feat: Svelte native HMR compatibility for NativeScript 8 #85

Merged
merged 5 commits into from
Feb 13, 2024

Conversation

CatchABus
Copy link
Contributor

@CatchABus CatchABus commented Oct 27, 2023

This PR focuses on getting rid of hackish solutions and achieving compatibility with NativeScript 8.

We are finally getting rid of replacePage-like functions inside HMR as NativeScript 8 has enhanced existing frame method to work on cases like this one.
Reference: NativeScript/NativeScript#9460

Regarding the view event handling interception, it seems that it's currently breaking navigateFrom events in general and on top of that the relay concept must have broken at a certain point.
We simplified this by keeping the needed event handler in an hmr-specific view property on svelte-native side.
In few words, we now achieve the same result without intercepting view methods.
Reference: halfnelson/svelte-native@9531967#diff-b2e62c5821c8b441c5f0e6e102a78fbe1b04496102330acfeb3e025552ca9b1cR82

These changes should let svelte-hmr work smoothly on svelte-native.

@CatchABus CatchABus changed the title Svelte native compatibility improvements for NativeScript 8 feat: Svelte native compatibility for NativeScript 8 Oct 27, 2023
@CatchABus CatchABus changed the title feat: Svelte native compatibility for NativeScript 8 feat: Svelte native HMR compatibility for NativeScript 8 Oct 29, 2023
@farfromrefug
Copy link

@benmccann i see you ve not been really working on this in the last few month. Are you still maintaining this repo?
we kind of really need that PR

@benmccann
Copy link
Member

I don't work on the HMR portion of Svelte, but can tell you that this repo is going away for Svelte 5 and HMR will be integrated directly into the core of Svelte. See sveltejs/svelte#10100

@farfromrefug
Copy link

@benmccann thanks for the reply. Could we still find a way to have this merge for it to work with svelte 4?
svelte 5 is not there yet and not all apps will migrate to svelte5 as soon as it lands. So this PR could help a lot of users for the months to come

@rixo
Copy link
Collaborator

rixo commented Feb 9, 2024

Sorry for being so long to give you an answer.

Unfortunately, I won't have the time to test this myself, or even review it properly.

@CatchABus @farfromrefug I see both of you are apparently maintaining svelte-native at the moment, and the diff shows that the change will only affect svelte-native... So I am willing to trust your judgement and just merge the change.

Is the MR ready for this, or do you need to make additional changes?

@CatchABus
Copy link
Contributor Author

CatchABus commented Feb 10, 2024

Sorry for being so long to give you an answer.

Unfortunately, I won't have the time to test this myself, or even review it properly.

@CatchABus @farfromrefug I see both of you are apparently maintaining svelte-native at the moment, and the diff shows that the change will only affect svelte-native... So I am willing to trust your judgement and just merge the change.

Is the MR ready for this, or do you need to make additional changes?

Thank you very much for the response. Yes, it's ready for merge.

Note: If HMR is moving into svelte 5 core, we might eventually have to find a way to move all this adapter logic to svelte-native so that we don't affect the release flow of svelte itself for every change we do. Might discuss this with @farfromrefug

Either way, I'm sure we'll find a good solution for what I mentioned above once v5 is officially out.
Thanks a lot!

@rixo rixo merged commit 283c6cb into sveltejs:master Feb 13, 2024
2 of 5 checks passed
@rixo
Copy link
Collaborator

rixo commented Feb 16, 2024

Published as [email protected].

I agree that the Native HMR adapter should probably live in the Native's repo for Svelte 5. I'm not too sure how Native will integrate with Svelte 5 though; have you started working on it yet?

@rixo
Copy link
Collaborator

rixo commented Feb 16, 2024

By the way, would you be able to tell me if #9 is still current?

@farfromrefug
Copy link

@rixo thanks for the release.
No we have looked at svelte 5 yet. Not sure if svelte 5 already implemented custom renderers? We will need that for svelte-native.
As for #9 to be honest i am not sure. Not sure i understand what i should do to try and replicate it. I need to disable debug mode for svelte? how would i do that?

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.

4 participants