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

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

Merged
merged 3 commits into from
Dec 11, 2023

Conversation

jhsware
Copy link
Contributor

@jhsware jhsware commented Dec 2, 2023

(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
Copy link
Member

Havunen commented Dec 3, 2023

testing if re-opening the PR triggers Travis

@Havunen Havunen closed this Dec 3, 2023
@Havunen Havunen reopened this Dec 3, 2023
@Havunen
Copy link
Member

Havunen commented Dec 3, 2023

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
Copy link
Member

Havunen commented Dec 3, 2023

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 Havunen closed this Dec 7, 2023
@Havunen
Copy link
Member

Havunen commented Dec 7, 2023

testing coveralls

@Havunen Havunen reopened this Dec 7, 2023
@Havunen Havunen closed this Dec 7, 2023
@Havunen Havunen reopened this Dec 7, 2023
@Havunen Havunen closed this Dec 7, 2023
@Havunen Havunen reopened this Dec 7, 2023
@Havunen Havunen closed this Dec 7, 2023
@Havunen Havunen reopened this Dec 7, 2023
@Havunen Havunen closed this Dec 7, 2023
@Havunen Havunen reopened this Dec 7, 2023
@Havunen Havunen closed this Dec 9, 2023
@Havunen Havunen reopened this Dec 9, 2023
@Havunen
Copy link
Member

Havunen commented Dec 10, 2023

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
Copy link
Member

Havunen commented Dec 11, 2023

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

@Havunen Havunen merged commit d05dc2d into infernojs:master Dec 11, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants