QwikDev / qwik

Instant-loading web apps, without effort
https://qwik.dev
MIT License
20.7k stars 1.29k forks source link

[🐞] Cannot access full url in useLocation hook #4767

Open CukierPo2zl opened 1 year ago

CukierPo2zl commented 1 year ago

Which component is affected?

Qwik City (routing)

Describe the bug

I am using client-side routing and navigating the page using id's and scrolling to the view. I navigate using the useNavigate hook and trying to access the id (or full url) value in useLocation. Unfortunatelly useLocation doesn't include information that starts with a '#'. eg.

const nav = useNavigate();
const handleClick = $(() => {
    nav(`#section`);
  });

After that url looks correctly

https://localhost:4200/#section

But cannot find any hash information

const loc = useLocation();
console.log(loc)
{
  hash: "",
  host: "localhost:4200",
  hostname: "localhost",
  href: "http://localhost:4200/",
  origin: "http://localhost:4200",
  password: "",
  pathname: "/",
  port: "4200",
  protocol:"http:",
  search: "",
  ...
}

any thoughts?

Reproduction

https://stackblitz.com/edit/qwik-starter-casjao?file=src%2Froutes%2Findex.tsx

Steps to reproduce

System Info

System:
    OS: macOS 13.0.1
    CPU: (8) arm64 Apple M1 Pro
    Memory: 77.41 MB / 32.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 19.3.0 - ~/.nvm/versions/node/v19.3.0/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 9.2.0 - ~/.nvm/versions/node/v19.3.0/bin/npm
  Browsers:
    Chrome: 114.0.5735.198
    Edge: 113.0.1774.57
    Safari: 16.1
  npmPackages:
    @builder.io/qwik: ~1.2.6 => 1.2.6 
    @builder.io/qwik-city: ~1.2.6 => 1.2.6 
    undici: ^5.22.0 => 5.22.1 
    vite: ^4.3.9 => 4.3.9

Additional Information

I think it might be affected by this PR https://github.com/BuilderIO/qwik/pull/4550

stackblitz[bot] commented 1 year ago

Fix this issue in StackBlitz Codeflow Start a new pull request in StackBlitz Codeflow.

jordanw66 commented 1 year ago

I worked on the client-side navigation, scroll restore, and recovery overhaul for 1.2. I can confirm that fragments are not in the useLocation.

The reason for this is because prior to 1.2, the state of SPA was extremely unpolished and unusable from a UX perspective, most of those issues have been fixed, but hashes in Link and useNavigate were actually already broken too and there were some challenges around making them work in a consistent way in Qwik's model.

The goal of Qwik is to be able to swap between Link and anchor tags freely. Because routeLocation is set during navigate, if someone uses anchor tags, instead of Link or useNavigate, with a hash then that won't register either.

This presents many challenges and edge cases around resuming, zero-JS upfront, anchor tags before navigate, etc.

You would and should be able to expect it to work the same in all situations but certain realities would mean you'd be able to scroll to a fragment id in different ways or at different times with useLocation updating in some but not others.

However, while it's not simple or straightforward to fix this, I am hoping to find a solution to these problems in a future update.

CukierPo2zl commented 1 year ago

@jordanw66 Thanks for the answer. I will try to workaround it. Will let you know if I find anything. Cheers!

gioboa commented 10 months ago

do you have any news about this issue?

tzdesign commented 5 months ago

@wmertens you were saying http is a problem in terms of server-side access, but useLocation should at least have the hash if you navigate with <Link/> or navigate right?

I have the problem, that it does not even work with window events like popstate or hashchange when the component is deep down.

Tested it with something like this:

export function useLocationHash() {
  const hash = useSignal('');

  // eslint-disable-next-line qwik/no-use-visible-task
  useVisibleTask$(({ track }) => {
    hash.value = window.location.hash;
  });
  useOnWindow(
    'hashchange',
    $(() => {
      hash.value = window.location.hash;
    })
  );
  return hash;
}

unfortunately this works with <a/> only. @jordanw66 isn't Link using navigate using push state?

My team has found more weird stuff going on, it looks like a clicks are prevented somehow after actions. We will create another issue with stackblitz and custom repo.

tzdesign commented 5 months ago

@gioboa @jordanw66 What do you think about the request to have useLocation know about hash? It's really weird from DX if you need to know that this is just the SSR delivered location, but has the structure window.location.

It would be also pretty neat in my opinion to do stuff on useLocation().url.hash without having something like my hook. Not sure if qwik at some point could update the location.

jordanw66 commented 4 months ago

@tzdesign Good question, this is a very complex issue. Link does indeed navigate using pushState.

The problem is QwikCity (QC) is at core an SSR framework. Any page can be one that was arrived at either via MPA or SPA. QC doesn't care, if it's not an SPA navigate it will SSR the page for the client.

Even with a page arrived at via SPA, when a refresh occurs, or the link is copied/shared and visited, the server will render a page. It has no idea nor does it care, there's no way to tell solely based on the URL received.

Because the browser doesn't send the URL fragment, the server has no way to know about this. This creates our first problem: if your app relies on the fragment, the server and browser are now out of sync and generating different content depending on how a page is arrived at.

How do we solve this? Do we reboot the client to re-render in this case? How do we know when to do this? Reconcile state? Is this going to play nicely with future features? Etc. Very complex.

There are so many edge cases to consider and hard problems to solve, it's almost an exponential explosion.

When I fixed the SPA in QC, fragments already didn't work so I left them out. The time it would've taken to solve this is probably equal or greater than all the time it took to solve all the other issues combined. Getting SPA working great was an immediate issue for me and time was limited, both for myself and the next Qwik release at the time.

It was my intention to attempt to solve this afterwards but I got very sick for a few months and ended up in the hospital. I haven't had time since.

I'm still not sure if this issue IS actually solvable to be honest, at least easily. How does Next.js handle fragments? From my understanding last time I checked it doesn't and using them requires the same kind of hacky workarounds using window.location on the client directly as well. This may have changed, if they figured it out I would start there for a solution.

tzdesign commented 4 months ago

@jordanw66 thanks for taking the time. I really hope you are ok again!

I understand the problem now and you might be right. What do you think about omitting has hash from the URL type? This way it's not confusing anymore. We could add it to the docs, that it's not possible for now.

Not sure about nextjs. We have a lot going on the product detail page. The current implementation is jQuery, so we can do what ever we want anyway. I have never looked into the nextjs project. But I think if we document it and possibly remove hash from the URL object, then it is at least understandable and a better DX.

jordanw66 commented 4 months ago

@tzdesign It might make sense for DX, you'd have to ask the core team members on an API decision like this. Documenting this is probably a good idea either way. Maybe even documenting an (albeit hacky) "best practice" on how to implement a good client-side workaround on this issue would be helpful.

The reason I use Next as an example is that Vercel is a huge company and that's a React-based framework with large teams of highly paid professionals working on it full-time. This problem seems extremely complex to me initially, and if they haven't been able to devise a nice solution then I think there's a good chance my intuition is correct and my ability to conceptualize and deliver an ergonomic solution that fits into the Qwik model, especially in my limited spare time, is probably unlikely.