Skip to content
This repository has been archived by the owner on Dec 30, 2022. It is now read-only.

fix(nextjs/ssr): parse params.asPath #189

Merged
merged 3 commits into from
Jul 25, 2017
Merged

fix(nextjs/ssr): parse params.asPath #189

merged 3 commits into from
Jul 25, 2017

Conversation

iam4x
Copy link
Contributor

@iam4x iam4x commented Jul 25, 2017

There is an edge case where params.asPath === '/' and gets converted to {'/': null} as search state.

Checking that we have an ? before fix this issue.

There is an edge case where `params.asPath === '/'` and
gets converted to `{'/': null}` as search state.
Checking that we have an `?` before fix this issue.
Copy link
Contributor

@Haroenv Haroenv left a comment

Choose a reason for hiding this comment

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

LGTM, support question

const searchState = qs.parse(
params.asPath.substring(params.asPath.indexOf('?') + 1)
);
const searchState = params.asPath.includes('?')
Copy link
Contributor

Choose a reason for hiding this comment

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

.includes is not supported everywhere we support. I think you should do indexOf('?') > -1 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this context with nextjs and babel is it safe to use .includes 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I forgot that this was node-only

@algobot
Copy link
Contributor

algobot commented Jul 25, 2017

Deploy preview ready!

Built with commit a3c96e0

https://deploy-preview-189--react-instantsearch.netlify.com

@mthuret mthuret merged commit ae17da0 into master Jul 25, 2017
@mthuret mthuret deleted the fix-nextjs-example branch July 25, 2017 15:58
mthuret pushed a commit that referenced this pull request Aug 3, 2017
<a name="4.1.0-beta.4"></a>
# [4.1.0-beta.4](v4.1.0-beta.3...v4.1.0-beta.4) (2017-08-03)

### Bug Fixes

* **deps:** Update dependency lint-staged to version ^4.0.0 (#201) ([6867a1b](6867a1b))
* **nextjs/ssr:** parse `params.asPath` (#189) ([ae17da0](ae17da0))
* **PoweredBy:** add a label to the Algolia logo (#216) ([cd235bd](cd235bd))
* **react:** remove typo around `"" 2` (#220) ([f73eb04](f73eb04))
* **ScrollTo:** scroll to only if change triggered by the widget observed (#202) ([2d76022](2d76022))
* **theme:** format the count of items appearing in a refinement (#217) ([2225c24](2225c24))
marielaures pushed a commit that referenced this pull request Aug 3, 2017
<a name="4.1.0-beta.4"></a>
# [4.1.0-beta.4](v4.1.0-beta.3...v4.1.0-beta.4) (2017-08-03)

### Bug Fixes

* **deps:** Update dependency lint-staged to version ^4.0.0 (#201) ([6867a1b](6867a1b))
* **nextjs/ssr:** parse `params.asPath` (#189) ([ae17da0](ae17da0))
* **PoweredBy:** add a label to the Algolia logo (#216) ([cd235bd](cd235bd))
* **react:** remove typo around `"" 2` (#220) ([f73eb04](f73eb04))
* **ScrollTo:** scroll to only if change triggered by the widget observed (#202) ([2d76022](2d76022))
* **theme:** format the count of items appearing in a refinement (#217) ([2225c24](2225c24))
marielaures pushed a commit that referenced this pull request Aug 7, 2017
<a name="4.1.0-beta.4"></a>
# [4.1.0-beta.4](v4.1.0-beta.3...v4.1.0-beta.4) (2017-08-03)

### Bug Fixes

* **deps:** Update dependency lint-staged to version ^4.0.0 (#201) ([6867a1b](6867a1b))
* **nextjs/ssr:** parse `params.asPath` (#189) ([ae17da0](ae17da0))
* **PoweredBy:** add a label to the Algolia logo (#216) ([cd235bd](cd235bd))
* **react:** remove typo around `"" 2` (#220) ([f73eb04](f73eb04))
* **ScrollTo:** scroll to only if change triggered by the widget observed (#202) ([2d76022](2d76022))
* **theme:** format the count of items appearing in a refinement (#217) ([2225c24](2225c24))
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants