(preact-iso): Use pushState/ replaceState for useLocation().route method

[piotr-cz]

At this moment when using useLocation().route('/foobar') window location doesn't change: neither history.pushState nor history.replaceState method is used.

This pull request

1. changes method behavior in that window location will be added to history via pushState.
2. allows using object as a parameter so it's possible to use replaceState

// Use history.pushState
route('/foobar);
route({ url: '/foobar' });
route({ url: '/foobar', replace: false });

// Use history.replaceState
route({ url: '/foobar', replace: true });

Originally the useReducer hook dispatcher had a third push argument, but dispatchers consume only two arguments.

wmr/packages/preact-iso/router.js

Line 4 in 58f1bff

const UPDATE = (state, url, push) => {

I'm not sure if I should bump patch or minor version with changeset?

[changeset-bot]

🦋 Changeset detected

Latest commit: 3d0d9f4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
preact-iso	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[developit]

It looks like this was already supported?

route('/url', true)

(admittedly, push=true might be a better default)

[developit]

Wondering if we can use (url, replace) here? That would match preact-router.

[piotr-cz]

@developit
Unfortunately passing replace flag as additional argument doesn't work, as route is actually a reducers dispatcher, which accepts only one argument (see docs > useReducer):

wmr/packages/preact-iso/router.js

Lines 44 to 51 in 58f1bff

	const [url, route] = useReducer(UPDATE, location.pathname + location.search);
	const value = useMemo(() => {
	const u = new URL(url, location.origin);
	const path = u.pathname.replace(/(.)\/$/g, '$1');
	// @ts-ignore-next
	return { url, path, query: Object.fromEntries(u.searchParams), route };
	}, [url]);

Probably we would have to refactor router code to not use reducer in this case (which may be not a bad idea to handle history outside/ without reducer).

[piotr-cz]

I prefer route(url, replace) as well, but as long as there is no better solution, let's keep this PR opened for reference

[piotr-cz]

I've created new PR in #424 to fix the bug with location change after using useLocation().route(path).

After it's resolved it will be easier to move forward with adding an option such as route(path, replace) either with this PR new one.

[developit]

Nice - thanks for pulling that out, just merged it.

[developit]

Here's what I'd suggest, in order to preserve the duck typing (as mentioned in previous comment):

const UPDATE = (state, update) => {
	/** @type {boolean|undefined} - History state update strategy */
	let push, url;

	if (!update || typeof update === 'string') {
		url = update;
		push = true;
	}
	else if (update.type === 'click') {
		// user click
		// @ts-ignore-next
		const link = update.target.closest('a[href]');
		if (!link || link.origin != location.origin) return state;

		update.preventDefault();
		url = link.pathname + link.search + link.hash;
		push = true;
	}
	else if (update.type === 'popstate') {
		// navigation
		url = location.pathname + location.search + location.hash;
	}
	else {
		// manual invocation: route(url) or route({ url, replace })
		url = update.url || update;
		push = !url.replace;
	}

	if (push === true) history.pushState(null, '', url);
	else if (push === false) history.replaceState(null, '', url);
	return url;
}

[piotr-cz]

Should we allow passing a value that evaluates to false (undefined/ null) with route method?
If so, then should it be added to history?

	if (!update || typeof update === 'string') {
	//  ^

When the update is not either string or event I'd say it must be an object?

	else {
		// manual invocation: route(url) or route({ url, replace })
		url = update.url || update;
		//                  ^

[piotr-cz]

I've updated PR with code you suggested, adapting to eslint/ prettier rules

[piotr-cz]

@developit
What do you think about my comments above?

[piotr-cz]

I've updated PR to use code suggested in #413 (comment)

However I'm not happy with the route({ url, replace }) signature (proposed by me) and would prefer route(url, replace) instead.
Let's not merge this PR yet, we can do better.

[developit]

@piotr-cz I think we might be able to create a wrapper function in the provider that swaps the argument signature:

-	const [url, route] = useReducer(UPDATE, location.pathname + location.search);
+	const [url, doRoute] = useReducer(UPDATE, location.pathname + location.search);
+	const route = useCallback((url, replace) => doRoute({ url, replace }), []);

It might even be possible to simplify the history event handling this way too.

[piotr-cz]

@developit
Updated, thank you for suggestion!

[piotr-cz]

Just one question:
Should I add changeset with patch or minor version bump?
As the route method is not documented, I guess patch is enough.

[JoviDeCroock]

I'd go for patch yea