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

docs(readme): Note added for history v5's types #387

Closed
wants to merge 1 commit into from

Conversation

rschristian
Copy link
Member

What kind of change does this PR introduce?
Expounding upon documentation

Summary
#385 brought up history v5's types being incompatible with preact-router's. If a new user were to read the custom history section and use the latest version of history along with TypeScript then they would not be able to build their application.

I figure that adding a note is a simple and quick temporary solution for anyone else who comes across this. Let me know if I should change the verbiage in any way.

@janybravo
Copy link

It is not only types from history@5 npm package that are incompatible. Also interfaces are incompatible causing incorrect behavior when using custom history@5 without an error. So I think is important to clarify that or even better to upgrade to latest history.

@rschristian
Copy link
Member Author

rschristian commented Dec 23, 2020

@janybravo Thanks for the extra info.

What do mean by "...or even better to upgrade to the latest history"? This lib doesn't actually use history, just used to be compatible. I'm not sure whether it's a goal to be compatible or not either, as undoubtedly this lib updates less frequently than history which would make it difficult to stay compatible as time went on.

@janybravo
Copy link

@rschristian. I know that is just and interface. I assumed that history that could be provided to Router via customHistory, should be compatible with history@5, by which I mean latest version of it.

I see what you mean. If it helps, I have PR ready to have preact-router work better with history@5 if it helps. It would break compatibility with history@4 since they changed their interface.

@rschristian
Copy link
Member Author

Support for v5 was added in #410

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

Successfully merging this pull request may close these issues.

2 participants