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

Router #68

Merged
merged 40 commits into from Oct 28, 2022
Merged

Router #68

merged 40 commits into from Oct 28, 2022

Conversation

armanbilge
Copy link
Owner

Pretty much done I think, just needs an example.

@armanbilge armanbilge marked this pull request as ready for review August 7, 2022 07:03
armanbilge added a commit that referenced this pull request Aug 7, 2022
@armanbilge
Copy link
Owner Author

@jberggg I hope you don't mind, your work on the fs2 router also inspired me to explore some ideas in this space 😃 if you have a chance I would really appreciate your feedback and ideas here 🙏 thanks in advance!

I was largely motivated by Calico's special requirements: unlike Outwatch, which uses a virtual DOM for rendering, the Calico DSL is in terms of the raw DOM. I wanted to make sure that Calico provides a router API that implements navigation without re-rendering the entire page (or a large portion of it), since this would be prohibitively bad for performance.

I've pre-published the docs:
https://armanbilge.github.io/calico/router.html

As well as a snapshot:

resolvers += Resolver.sonatypeOssRepos("snapshots")
libraryDependencies += "com.armanbilge" %%% "calico-router" % "0.1.1-99-27bfdf3-SNAPSHOT"

@jberggg
Copy link

jberggg commented Aug 8, 2022

Hey Arman 👋🏽 We just moved so there was not much time to work on my router mini project. Will push it to a first release as soon as I can. I don't mind your specialised router and I will have a look at this PR tomorrow, today is a busy day for me 🧑‍💼

Copy link

@AesaKamar AesaKamar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool!

Sidenote: I've been really enjoying working with this project so far~

def replaceState(state: S, url: URL): F[Unit]

object History:
def make[F[_], S: Decoder: Encoder](using F: Async[F]): Resource[F, History[F, S]] =

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still learning some Scala3 idioms...

Is it common to use this the F name to bind the implicit instances?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! I would say this is a Typelevel/Cats idiom, not specific to Scala 3 :)

object History:
def make[F[_], S: Decoder: Encoder](using F: Async[F]): Resource[F, History[F, S]] =
Dispatcher[F].map { dispatcher =>
new:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this syntax do?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new: ? This is Scala 3 shorthand for new History[F, S] { ... }

Comment on lines 36 to 50
sealed trait History[F[_], S]:

def state: Signal[F, S]
def length: Signal[F, Int]

def forward: F[Unit]
def back: F[Unit]
def go: F[Unit]
def go(delta: Int): F[Unit]

def pushState(state: S): F[Unit]
def pushState(state: S, url: URL): F[Unit]

def replaceState(state: S): F[Unit]
def replaceState(state: S, url: URL): F[Unit]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this API, very simple and clean!

I've seen some other implementations like in https://github.com/raquo/scala-dom-types to link out to the underlying MDN docs as well

Do you think it makes sense to do that?

Does it also make sense to add some scaladoc to this (outside of the awesome conceptual overview in the readme) ?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, scaladocs are a bit overdue 😅 . Linking to MDN seems good :)

given Monoid[F[Option[Route[F]]]] with
def empty = F.pure(None)
def combine(x: F[Option[Route[F]]], y: F[Option[Route[F]]]) =
(x, y).mapN(_.orElse(_))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been looking for a simple pattern that's similar to <|> from Haskell's Alternative

I like this, I'mma start using it (:

Comment on lines 36 to 37
def navigate(uri: Uri): F[Unit]
def teleport(uri: Uri): F[Unit]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the intended differences between these 2 method?

Otherwise, they looks kind of like synonyms (as I'm currently reading it)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, another one need scaladoc. Feel free to suggest better names!

navigate is like pushState: you change location, and also push it into your browser history.

teleport is like replaceState: you change location wihtout pushing into your browser history, instead replacing the current entry.

https://developer.mozilla.org/en-US/docs/Web/API/History/replaceState

@armanbilge
Copy link
Owner Author

+155.32KB +80.52% total files change

Sad 😕 this is probably mostly http4s Uri plus its parser. In any case, in a real application these would get dragged in by the FetchClient in http4s-dom.

@armanbilge armanbilge merged commit 9fed01a into main Oct 28, 2022
@armanbilge armanbilge deleted the pr/router branch October 28, 2022 03:39
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.

None yet

3 participants