search

infernojs / inferno

:fire: An extremely fast, React-like JavaScript library for building modern user interfaces
https://infernojs.org
MIT License
16.1k stars 635 forks source link

Added tests for SSR with inferno-router and fix bug in <Switch> (rebased on master) #1658

Closed jhsware closed 11 months ago

jhsware commented 11 months ago

(rebased on master, all tests passing)

Using Switch with inferno-router was missing tests for SSR. Adding them revealed a bug where all matching routes that are children of Switch would be returned by traverseLoaders. Whereas the UI would render correctly, this would cause unnecessary calls to API:s.

Also, tightened check that path matches are only done on Route Components and not any Component that looks like a route. The current behaviour could cause unexpected behaviour.

A sidenote: While implementing function _isRoute(node: any) I was surprised that I needed a fallback check for inheritance of Route when routes where passed as an array (see comment in code). This feels lika an inconsistency in Inferno where array and component implementation gives slightly different results. Just wanted to mention it, I haven't investigated any further.

Havunen commented 11 months ago

testing if re-opening the PR triggers Travis

Havunen commented 11 months ago

There are some TS lint errors in the build log, can you check please?

https://app.travis-ci.com/github/infernojs/inferno/builds/267615174

Havunen commented 11 months ago

A sidenote: While implementing function _isRoute(node: any) I was surprised that I needed a fallback check for inheritance of Route when routes where passed as an array (see comment in code). This feels lika an inconsistency in Inferno where array and component implementation gives slightly different results. Just wanted to mention it, I haven't investigated any further.

Yes, Since the early days of Inferno it has been so that if in JSX one node has a single children it is set as the children prop directly, if there are multiple childrens in the give node then it becomes an array.

Havunen commented 11 months ago

testing coveralls

Havunen commented 11 months ago

I contacted coveralls support, for some reason it is not sending the pull request comments from the service, but shows the statistics in their web app

Havunen commented 11 months ago

I will merge this as the change seems good to me. Lets check that coveralls issue another time.