preactjs / preact-router

:earth_americas: URL router for Preact.
http://npm.im/preact-router
MIT License
1.01k stars 156 forks source link

preact-router not compatible with history v5 #423

Open daniel-j-h opened 2 years ago

daniel-j-h commented 2 years ago

Hey folks,

I'm on preact-router@4.0.1 and history@5.3.0 (both latest stable versions), and I'm running into similar / the same issues as reported in https://github.com/preactjs/preact-router/issues/385. I was looking around a bit and found https://github.com/preactjs/preact-router/pull/410 and https://github.com/preactjs/preact-router/pull/387 which claim to have solved the history v5 issue.

I have checked my package and package-lockfile to make sure the versions installed are the versions reported above.

Here is the use case and the error message, in case it helps

import { createHashHistory } from 'history';

// ...

<Router history={createHashHistory()}>
src/components/app.tsx:20:17 - error TS2322: Type 'HashHistory' is not assignable to type 'CustomHistory'.
  Types of property 'listen' are incompatible.
    Type '(listener: Listener) => () => void' is not assignable to type '(callback: (location: Location) => void) => () => void'.
      Types of parameters 'listener' and 'callback' are incompatible.
        Types of parameters 'location' and 'update' are incompatible.
          Type 'Update' is missing the following properties from type 'Location': pathname, search

20         <Router history={createHashHistory()}>
                   ~~~~~~~

  node_modules/preact-router/index.d.ts:46:2
    46  history?: CustomHistory;
        ~~~~~~~
    The expected type comes from property 'history' which is declared here on type 'IntrinsicAttributes & RouterProps<Record<string, string | undefined> | null> & Readonly<Attributes & { children?: ComponentChildren; ref?: Ref<...> | undefined; }>'

Workarounds I tried (and don't work)

  1. Downgrading to history@4 does not work, because it doesn't seem to come with typescript types, and tsc type checking then fails.

  2. Using history@5 with the workaround from https://github.com/preactjs/preact-router/issues/385#issuecomment-894076265 type-checks, but then at build time results in a very strange non-descriptive error message.

 Build  [=========== ] 95% (13.6s) emitting
ReferenceError: document is not defined
method: null
at: undefined:null:null

Source code:

✖ ERROR TypeError [ERR_INVALID_ARG_TYPE]: The "chunk" argument must be of type string or an instance of Buffer or Uint8Array. Received undefined
    at new NodeError (node:internal/errors:371:5)
    at _write (node:internal/streams/writable:312:13)
    at WriteStream.Writable.write (node:internal/streams/writable:334:10)
    at handlePrerenderError (/app/node_modules/preact-cli/lib/lib/webpack/prerender.js:110:18)

Any ideas what could be the cause of this? Curious if other folks successfully are using latest preact-router and history v5 with typyescript. Let me know if I can provide more information here.

rschristian commented 2 years ago

Well, types not lining up doesn't mean it's incompatible necessarily. Ideally they would, but TS type incompatibilities are pretty common in the ecosystem as a whole.

Now, if you get runtime errors then absolutely. You might need to configure your tsconfig.json to be more lenient or simply use // @ts-ignore.

Downgrading to history@4 does not work, because it doesn't seem to come with typescript types, and tsc type checking then fails.

This is purely a result of your strict configuration; you can disable noImplicitAny or, again, use ignore comments.

It does have types at @types/history, which might be usable (DefinitelyTyped can be a bit hit-or-miss).

but then at build time results in a very strange non-descriptive error message.

The error tells you exactly what the problem is: document is not defined.

This is the same issue as #414, and maybe there's a disconnect here? Hash routers only work in a browser environment, therefore you cannot use them in Node, which preact-cliuses in order to prerender your apps. They're incompatible.

You can get around this by dropping out of prerendering or using window checks (see https://github.com/preactjs/preact-cli#pre-rendering). However, without the router being available, there's likely little value in prerendering.

rxliuli commented 4 months ago

Well, types not lining up doesn't mean it's incompatible necessarily. Ideally they would, but TS type incompatibilities are pretty common in the ecosystem as a whole.

Now, if you get runtime errors then absolutely. You might need to configure your tsconfig.json to be more lenient or simply use // @ts-ignore.

You're right, this kind of type error sometimes occurs in the TypeScript ecosystem. However, it has been two years since the last release of the history module. Perhaps it would be better if the type definitions of preact-router could be modified so that no one has to worry about this error? Especially since adding a preact-router dependency directly from the vite+preact template for use in TypeScript will lead to this error. of course, I can always solve it by using as any, no matter what.

rschristian commented 4 months ago

PRs welcome, but I'd push people to use preact-iso instead.

reddec commented 1 month ago

@rschristian preact-iso doesn't support hash-based routing (and not planned based on issues)

rschristian commented 1 month ago

Indeed, so perhaps that comment was not super relevant, though I would urge people to really consider whether they need hash routing. By and large, they're a relic with very few uses today.

daniel-j-h commented 1 month ago

The use case I had in mind when I originally opened the ticket was deploying a preact page on a static host such as Github Pages or AWS S3. On there you'd deploy a single index.html plus assets such as css, js, etc.

And on these deployments it makes a difference if the route is

because in the first case if a user goes to that route, the static page server will respond with an error page and/or error response code even before the preact code has any chance to run. In the second case the user goes to index.html where the route gets properly handled by the preact hash router.

On some deployments there are workaround but on others there are not; and there's also a high chance that I might not fully understand how such a preact deployment without a hash router is supposed to look like.

Just adding this context because from this use case's point of view a hash router is definitely needed. Is this not a use case other people are running into and am I just using this wrong?

smdrager commented 1 month ago

I'll also throw in hash routing is useful for local/standalone deployment scenarios as well. Having the ability to do both means you routing can "just work" regardless of whether you control the server, or if it is even on a server. While these are not the most common use case, they are not uncommon either.

rschristian commented 1 month ago

preact-router isn't going anywhere, it can continued to be used for any hash routers people need, as can the ecosystem of alternative routers. preact-iso looks to offer isomorphic tooling and hash routers are inherently non-isomorphic, which is why it will not be offered there at any point. It just wouldn't make sense and would clash with some of the tools offered.

Ideally those static hosts that are still stuck in the 90's will catch up though -- Nginx & Apache have offered fallback routes for 20+ years, it's quite embarrassing that some hosts that are unable to do the same. That just isn't an issue you'll run into on a properly established web host.

smdrager commented 1 month ago

That is of course your prerogative. I only wanted to voice support for the feature as it is something I'd use myself.