mefechoel / svelte-navigator

Simple, accessible routing for Svelte
Other
506 stars 39 forks source link

changes to work with svelte 4. #112

Open norricorp opened 11 months ago

norricorp commented 11 months ago

Tested in my application but is there a test suite?

norricorp commented 11 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 4 months ago

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 commented 5 days ago

@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 commented 4 days ago

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.