[Bug]: Landing page links should use router

[kgryte]

The landing/welcome page links to documentation pages should use <Link> elements to allow navigation within the SPA.

Currently, when clicking on a documentation page, the application reloads.

[Planeshifter]

@kgryte Related issue: Links in the landing page to other packages sometimes are broken due to missing version information

Steps to reproduce: 1) Hard refresh of https://stdlib.io/docs/api/latest 2) Do a normal refresh by clicking the reload button of the browser 3) Click on one of the package links; they will look like this: https://stdlib.io/docs/api/undefined/@stdlib/random/base/

Proposed solution: If the version is missing, should we impute latest instead of going with undefined.

[kgryte]

Thanks for noticing this!

[kgryte]

Hmm...I am not able to reproduce on my end.

[kgryte]

Yeah, I cannot get the version to be undefined. And that it would be so is a bit odd, as props.version should always be defined.

[Planeshifter]

Hmm, maybe I didn't have the latest version of the docs website cached yet. Cannot reproduce anymore, see below gif for how the problem looked like.

[Planeshifter]

However, even though not reproducible, we could act defensively and add a default property to the <Welcome /> component:

Welcome.defaultProps = {
    'version': 'latest'
};

Adding sensible defaults in case data is missing may be a good idea to do more generally .

I also noticed that we do not use PropTypes for run-time property type validation during development. It's best practice to use such type checking in React projects and am wondering why we didn't do so (I assume we must have had a Slack chat about it).

[kgryte]

Re: PropTypes. This is the first I am hearing about it, and I don't recall any discussions. Not opposed to using in the application.

Re: defaultProps. Somewhat leery about doing this too much, as defaults, even if sensible, can mask bugs. For these components, we are assuming that the version propagates down the component stack. If it isn't, that tells me that the assumption is being violated, and, if things just "work", may be hard to track down where, when, and how that assumption is violated.

[Planeshifter]

See https://github.com/stdlib-js/www/commit/23fdbc6e9fe22dbe3434efc4fe62c0b57768b361 for an example of added property types. I would be happy to add them throughout the app. It's quite helpful for debugging during development and all bigger React projects I know of make use of it.

[kgryte]

As these are runtime checks, we probably first need to add tooling support to allow building the app in development mode. Currently, I think we always build in production mode.

[Planeshifter]

Nicer would be not having to build at all but use react-dev-server for hot module reloading.

[kgryte]

Not sure what is involved with setting that up. You have experience with it?

[Planeshifter]

Let me fiddle with it!

[kgryte]

🙏

[kgryte]

Re: propTypes. To be consistent with elsewhere, for static properties, added a JSDoc comment: https://github.com/stdlib-js/www/commit/2f604eb172440809478fe8eaf4888a09cb33881f.

Otherwise, if you want to add those across the application, would be appreciated!

[Planeshifter]

Sounds good, will do!

[kgryte]

🙌

[kgryte]

@Planeshifter Am still unable to reproduce the undefined version bug. Not clear to me where this might be coming from.

In client.jsx, we explicitly check for the absence of a version in the URL and fallback to a default value which is then threaded throughout the application:

https://github.com/stdlib-js/www/blob/78ff87f708d927953a6cb7959f9f957a49856bb1/src/client.jsx#L84-L87

For SSR, the route we'd be hitting above is /docs/api/:version, but in that route, we have access to a version URL parameter. I've confirmed locally that the version is correctly inserted into the URLs.

https://github.com/stdlib-js/www/blob/78ff87f708d927953a6cb7959f9f957a49856bb1/lib/server/lib/routes/version/version.js#L58

So not clear to me where things are going awry. Tis possible that this is a hydration issue of some kind when loading from the service worker.

[kgryte]

Ah...wait. Found a potential issue source for things to not be passed down correctly. Namely, with react-router.

Update: 🤷‍♂️ I dunno. React router should also be matching /docs/api/:version in order to render the welcome page.

https://github.com/stdlib-js/www/blob/78ff87f708d927953a6cb7959f9f957a49856bb1/src/routes.js#L34

https://github.com/stdlib-js/www/blob/78ff87f708d927953a6cb7959f9f957a49856bb1/src/app.jsx#L998-L1002

https://github.com/stdlib-js/www/blob/78ff87f708d927953a6cb7959f9f957a49856bb1/src/app.jsx#L719

[kgryte]

Followed the same steps as outlined above--hard refresh followed by refresh--and logged the value of match.params.version in _renderWelcome and all logged values are latest.

As this seems elusive to reproduce and there aren't any obvious code paths where version can be undefined, will go ahead and move on. Can always revisit in a separate issue thread if we run across this again.

[kgryte]

I should state, wrt the OP, the landing page now uses <Link> elements for internal routing.