Skip to content
This repository was archived by the owner on Feb 15, 2022. It is now read-only.

Model route and url types properly #216

Closed
2 of 5 tasks
ghost opened this issue Mar 25, 2021 · 13 comments
Closed
2 of 5 tasks

Model route and url types properly #216

ghost opened this issue Mar 25, 2021 · 13 comments
Assignees

Comments

@ghost
Copy link

ghost commented Mar 25, 2021

To the extent that it is practical, let's use shared constants, types, and validation logic to ensure that all URL data is a validly formed URL. For URLs to refer to pages within this site, let's also ensure that the URLs always point to valid pages. For URLs referring to external sites, consider ways to validate that the external site exists at site build time.

Some possible tasks:

  • Define global constant values for each internal URL. Whenever a page links to an internal URL, make use of these constant values. (Done in Define and use constants for internal urls #327)
  • Use a javascript library to validate that a string is a valid URL and make use of this validation when receiving URL values (e.g. when converting markdown to html or when parsing data from yaml files). Alternatively, we can assume all URL values are valid and that ood has already performed this validation.
  • At site build time, whenever we receive an external URL value, we could perform a live fetch of the url to check the http response code.
  • We could validate any internal URLs mentioned in markdown files against a set of valid internal URLs when converting the markdown to html. I don't expect links between tutorials. The tutorial navigation will steer users between tutorials.
  • We could use some form of code generation or more complex use of types to ensure only valid internal URLs are used.

Time box: 6 hours

@ghost ghost self-assigned this Mar 25, 2021
@agarwal agarwal unassigned ghost Apr 22, 2021
@ghost
Copy link
Author

ghost commented May 7, 2021

Some of the link validation might happen in ood linting - ocaml/ood#3.

@ghost
Copy link
Author

ghost commented May 13, 2021

This issue may demonstrate the worst case for an internal url change: #339 .

@ghost
Copy link
Author

ghost commented May 13, 2021

https://www.spock.li/2015/04/19/type-safe_routing.html and servant demonstrate examples of using centralized, compiler checked definitions of internal urls.

@rdavison
Copy link
Contributor

rdavison commented May 14, 2021

Spock/Servant are both very nice but afaik, we don't have something like that in the ocaml ecosystem. However, quite literally as of 2 days ago, the well typed router library was announced on the discuss forum. I haven't looked into it yet, but it's certainly worth investigating.

As for my initial thoughts on this issue, I think it's great that we have factored out the url constants as a first step, but I think we could take it further and create a new type to model internal urls. For example, I was thinking something more akin to the following:

type t = [
  | `Home
  | `Blog_post of int
]

val of_string : string -> t
val to_string : t -> string

One place I see this breaking down is when the URL path contains additional things, such as /blog-post/42/fr. If that's the case we might need to expand the type to something like:

type t =
  { lang : Lang.t option
  ; path : [ `Home | `Blog_post of int ]
  }

Or model the route in a different way.

@agarwal
Copy link
Member

agarwal commented May 19, 2021

One place I see this breaking down ...

I don't see why that's a break down. Seems perfectly fine to me.

lang : Lang.t option

Should it be required? Every page has to be in some language, with en as the default. Maybe. I'm not sure.

@agarwal
Copy link
Member

agarwal commented May 19, 2021

Review of #327:

  • I don't like the module name InternalUrls. Should be something more elegant like Url, and that module can have a type, general purpose constructors, the constants in InternalUrls, and a <Link> component (so maybe the module name should be Link).

@ghost
Copy link
Author

ghost commented May 19, 2021

Another word for an internalurl can be a route. We can think of url as one possible serialization of a route.

@ghost ghost changed the title Model url type properly Model route and url types properly May 22, 2021
@ghost
Copy link
Author

ghost commented Jun 10, 2021

It would be nice to have some specific internal and external url categories like webpage, image, and video. I would use the image url as part of abstracting parameters that will be useful in reusable components.

@ghost
Copy link
Author

ghost commented Jul 6, 2021

If we enumerate all the image url's as well, then we can avoid the need for scripts like the following: https://github.com/ocaml/v3.ocaml.org/pull/471/files#diff-62b8d055d3f221a52d60fcca16f433db5fefa7a87e41423d96b6ff74f21c45a4.

@rdavison
Copy link
Contributor

rdavison commented Jul 7, 2021

Merged ocaml/ocaml.org#470 to parameterize links by language.

@ghost
Copy link
Author

ghost commented Jul 10, 2021

Alternatively, we can assume all URL values are valid and that ood has already performed this validation.

Let's go with this assumption.

@ghost
Copy link
Author

ghost commented Jul 10, 2021

At site build time, whenever we receive an external URL value, we could perform a live fetch of the url to check the http response code.

Let's skip this.

@ghost
Copy link
Author

ghost commented Jul 23, 2021

We exceeded the time box and have made some improvements in this area. We can create a new issue if there are concrete refinements.

@ghost ghost closed this as completed Jul 23, 2021
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants