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

changes to work with svelte 4. #112

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

norricorp
Copy link

Tested in my application but is there a test suite?

@norricorp
Copy link
Author

Hi @mefechoel,
I run the tests (npm test) and the cypress tests are failing. For instance it can't get various id's.
I have also run the test app manually using cy:start and the links are not working so I need to investigate this. I was hoping that changing the package.json was enough but obviously not.
I compared to the original code and that works fine.
So currently confused.

@norricorp
Copy link
Author

norricorp commented Aug 24, 2023

Something not quite right is happening here.
I have started the test app (npm run cvy:serve) then open browser at localhost:7070. See test app.
The reason the cypress tests fail is because

		<Route path="about">
			<div data-testid="route-about">ABOUT</div>
		</Route>

in Main.svelte is not firing.
But here is the weird thing. If I update Main.svelte (or Memory?.svelte) to change some text eg a link text, then this does not show up in the browser. Even if I restart server (or build / start) then no change.
Anyone, any ideas?

@norricorp
Copy link
Author

Hi @mefechoel,
some help would be appreciated.
The cypress tests are failing as I explained above.
I noticed in Main.svelte in the testapp that you have the following lines

function appChange(state) {
        window.appState = state;
}

<Router disableInlineStyles>
       <LocationChange onChange={appChange} />
       <nav>

Should the onChange call to appChange have a parameter of state?

I wonder if svelte 4 is stricter. In svelte 3, clicking on an about link changes the url and that causes the Route tag to do a conditional render. But in svelte 4 this is not happening. An enter or refresh does cause a conditional render. But that is an extra step.

I added a console.log in appChange and clicking a link does not cause it to fire. Should it fire on a link click?

@norricorp
Copy link
Author

Bit more information.
Here is the console.log for navigator using svelte 3 where I have added various logging. Clicking on the about link from home
NORRIS: LINK: calling navigate
NORRIS: history: getLocation: returned object is {"href":"http://localhost:7070/about","origin":"http://localhost:7070/","protocol":"http:","host":"localhost:7070","hostname":"localhost","port":"7070","pathname":"/about","search":"","hash":"","state":{"_key":"jedb1y6uvmq"},"_key":"jedb1y6uvmq"}
NORRIS: LINK3: $location is now {"pathname":"/about","hash":"","search":"","state":{"_key":"jedb1y6uvmq"}}
NORRIS: LINK4: href is now "/about"
NORRIS: Router: history has changed {"location":{"href":"http://localhost:7070/about","origin":"http://localhost:7070/","protocol":"http:","host":"localhost:7070","hostname":"localhost","port":"7070","pathname":"/about","search":"","hash":"","state":{"_key":"jedb1y6uvmq"},"_key":"jedb1y6uvmq"}}

And the same using svelte 4
NORRIS: LINK: calling navigate
NORRIS: history: getLocation: returned object is {"href":"http://localhost:7080/about","origin":"http://localhost:7080","protocol":"http:","host":"localhost:7080","hostname":"localhost","port":"7080","pathname":"/about","search":"","hash":"","state":{"_key":"wezd50hjx4"},"_key":"wezd50hjx4"}
NORRIS: LINK3: $location is now {"pathname":"/","hash":"","search":"","state":null}
NORRIS: LINK4: href is now "/about"

So $location is not updated. Plus we do not get to Router in order to change history

@alexandermontillarivera
Copy link

alexandermontillarivera commented Sep 5, 2023

At least temporarily, you can override svelte-navigator dependency on svelte in package.json to use the library because it does work on svelte 4.

  "overrides": {
    "svelte-navigator": {
      "svelte": ">=4.x"
    }
  }

@norricorp
Copy link
Author

I think there is something wrong with the test app. I have run navigator with svelte 3 and 4 in parallel and the returned object from createHistory is the same in both but code like notifyListeners is not called in the svelte 4 version.

I know my javascript skills are not good enough to work out is going on here. I think as a short term fix, @alexandermontillarivera has the right idea and for longer term then moving to sveltekit is the answer.

@MrBns
Copy link

MrBns commented Mar 9, 2024

Why this Repository is in Pending for a longtime.. @mefechoel if you think you can't mantain this repo. please transfer ownership to someone who can actively manage.

@norricorp
Copy link
Author

@MrBns - but who can actively manage? I can't get the tests to pass though the changes allow my svelte 4 app to run. But I can't work out if the tests are no longer relevant or are finding a deep gotcha in the changes.

@norricorp
Copy link
Author

Would anyone like to work with me on this. I would really like to get this to pass the tests.
I am currently moving a sample application that I built with svelte and svelte-navigator to sveltekit and it is so hard.
So svelte-navigator does one job and that is the job I want; the rest I don't need.

Tests now run - all passed
Problem was rollup in test app
eslint - svelte3 -> svelte4
I also added examples to eslint ignore because problems had been skipped by using ignore eslint line in file which is not working with the replacement eslint-plugin-svelte
Prettier files changed too
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