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

useUrl #770

Open
huseeiin opened this issue Mar 10, 2025 · 12 comments
Open

useUrl #770

huseeiin opened this issue Mar 10, 2025 · 12 comments

Comments

@huseeiin
Copy link

Describe The Problem To Be Solved

on the server getRequestEvent() has Request.url but its not a convenient URL object, and on the client we have location which is not exactly the same as URL

Suggest A Solution

import { getRequestEvent, isServer } from "solid-js/web";

function useUrl() {
  return new URL(isServer ? getRequestEvent()!.request.url : location.href);
}

should this be added? just a suggestion!

@atk
Copy link
Member

atk commented Mar 11, 2025

How about making it reactive in the client (on history change event)?

@huseeiin
Copy link
Author

How about making it reactive in the client (on history change event)?

awesome idea!

@atk
Copy link
Member

atk commented Mar 11, 2025

But in a composable way, so keep useUrl and add createUrl.

@huseeiin
Copy link
Author

But in a composable way, so keep useUrl and add createUrl.

But in a composable way, so keep useUrl and add createUrl.

function useUrl() {
  const [currentUrl, setCurrentUrl] = createSignal(
    new URL(isServer ? getRequestEvent()!.request.url : location.href)
  );

  createEffect(() => {
    const handleLocationChange = () => {
      setCurrentUrl(new URL(location.href));
    };

    addEventListener("popstate", handleLocationChange);
    addEventListener("hashchange", handleLocationChange);

    return () => {
      removeEventListener("popstate", handleLocationChange);
      removeEventListener("hashchange", handleLocationChange);
    };
  });

  return currentUrl;
}

updated code with help from claude. you can add anything else you want.

@atk
Copy link
Member

atk commented Mar 11, 2025

Claude still treats Solid as if it was React. Instead of returning the unsubscriber, you need to wrap it in onCleanup.

@huseeiin
Copy link
Author

Claude still treats Solid as if it was React. Instead of returning the unsubscriber, you need to wrap it in onCleanup.

createEffect(() => {
  const handleLocationChange = () => {
    setCurrentUrl(new URL(location.href));
  };

  addEventListener("popstate", handleLocationChange);
  addEventListener("hashchange", handleLocationChange);

  onCleanup(() => {
    removeEventListener("popstate", handleLocationChange);
    removeEventListener("hashchange", handleLocationChange);
  });
});

is this correct?

@huseeiin
Copy link
Author

we don't need an effect here, should i just change it to an onMount?

@atk
Copy link
Member

atk commented Mar 11, 2025

Since there are no reactive properties inside the effect, it is actually the same, but onMount is shorter, so go for it.

@huseeiin
Copy link
Author

Since there are no reactive properties inside the effect, it is actually the same, but onMount is shorter, so go for it.

ok can you create createUrl? i don't know what is that, and i see you use createHydratableSingletonRoot instead of normal createSignal what else do i need to make this a real primitive?

@atk
Copy link
Member

atk commented Mar 11, 2025

createHydratableSignal is only meant for signals that are supposed to have different initial values on the server and on the client during hydration. Since our signal should basically have the same initial value, it is not necessary here.

In addition, we should use a equals function in the signal to avoid triggering updates when the URL itself is the same, only wrapped in a new object.

function useUrl() {
  return new URL(isServer ? getRequestEvent()!.request.url : location.href);
}

function createUrl() {
  const [currentUrl, setCurrentUrl] = createSignal(
    useUrl(),
    { equals: (a, b) => a.toString() === b.toString() }
  );

  onMount(() => {
    const handleLocationChange = () => {
      setCurrentUrl(new URL(location.href));
    };

    addEventListener("popstate", handleLocationChange);
    addEventListener("hashchange", handleLocationChange);

    onCleanup(() => {
      removeEventListener("popstate", handleLocationChange);
      removeEventListener("hashchange", handleLocationChange);
    });
  });

  return currentUrl;
}

@huseeiin
Copy link
Author

createHydratableSignal is only meant for signals that are supposed to have different initial values on the server and on the client during hydration. Since our signal should basically have the same initial value, it is not necessary here.

In addition, we should use a equals function in the signal to avoid triggering updates when the URL itself is the same, only wrapped in a new object.

function useUrl() {
return new URL(isServer ? getRequestEvent()!.request.url : location.href);
}

function createUrl() {
const [currentUrl, setCurrentUrl] = createSignal(
useUrl(),
{ equals: (a, b) => a.toString() === b.toString() }
);

onMount(() => {
const handleLocationChange = () => {
setCurrentUrl(new URL(location.href));
};

addEventListener("popstate", handleLocationChange);
addEventListener("hashchange", handleLocationChange);

onCleanup(() => {
  removeEventListener("popstate", handleLocationChange);
  removeEventListener("hashchange", handleLocationChange);
});

});

return currentUrl;
}

sounds awesome! is that all?

@huseeiin
Copy link
Author

small detail: should we call it createUrl/useUrl or createURL/useURL 🤔

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

No branches or pull requests

2 participants