Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

Fix #146 : add a way to make queries that don't change the inner state #193

Merged
merged 8 commits into from Sep 1, 2015

Conversation

bobylito
Copy link
Contributor

Introducing searchOnce, a method to query an index without modifying the state stored in the helper.

Alexandre Stanislawski added 5 commits August 21, 2015 13:04
This has been made in order to be able to reuse the way the requests for
the client without relying on a complete helper. This way we can create
requests just based on the search state.
```js
var state =
helper.searchOnce(
helper.state.addFacetRefinement('facet', 'f2');,
Copy link
Contributor

Choose a reason for hiding this comment

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

small typo, trailing ;

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I would start with a simpler example instead of calling addFacetRefinement, something like:

helper.searchOnce({hitsPerPage: 1}, function(error, content, state) { /*... */ });

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok makes sense. Actually I think this way of expressing the mutation of the state is nice but it involves creating a full new search state each time we call one of the methods which is not that wise...

@redox
Copy link
Contributor

redox commented Aug 24, 2015

Since searchOnce is calling all the _getQueries, can I avoid the underlying requests by disabling the faceting with:

// search for the 3 first best (distinct) hits
helper.searchOnce({query: 'foo', disjunctiveFacets: [], facets: [], distinct: true, hitsPerPage: 3}, fn);

@bobylito
Copy link
Contributor Author

Since searchOnce is calling all the _getQueries, can I avoid the underlying requests by disabling the faceting

Do you mean you want to avoid any other queries other than the main one (the one that does the search with all the filters)?

Alexandre Stanislawski added 3 commits August 26, 2015 14:15
Conflicts:
	dist/algoliasearch.helper.js
	dist/algoliasearch.helper.min.js
	src/algoliasearch.helper.js
@redox
Copy link
Contributor

redox commented Aug 29, 2015

Do you mean you want to avoid any other queries other than the main one (the one that does the search with all the filters)?

Yes, because most of the time the "extra" query we want to do is only to fetch "special" hits (like a query retrieving the top 3 products with promoted=1 or so).

@vvo
Copy link
Contributor

vvo commented Aug 30, 2015

Yaw.

About searchOnce, I did not explain why It was not my favourite naming: because the EventEmitter we have on the helper already introduce the once name:

helper.once('result');

So I thought that searchOnce could be understood as something related to the once of EventEmitter.

@bobylito
Copy link
Contributor Author

Yes, because most of the time the "extra" query we want to do is only to fetch "special" hits (like a query retrieving the top 3 products with promoted=1 or so).

From what I see in the code, we do send the refinements wether or not we have facets or disjunctive facets set. So if the engine let us do that then we're good :)

So I thought that searchOnce could be understood as something related to the once of EventEmitter.

Ok I get it! I don't think it'll be that much of a problem because we are assuming people are reading the documentation to get information about the API and we don't put too much emphasise on once and it'll also be in different sections of the README/cheat sheet. On the other hand search+once is semantically very accurate for the purpose of this method, imho.

@vvo
Copy link
Contributor

vvo commented Aug 31, 2015

I don't think it'll be that much of a problem

Yes it should be ok.

we don't put too much emphasise on once

Exactly

@bobylito
Copy link
Contributor Author

So after all, are we good to go on this PR? :)

@redox
Copy link
Contributor

redox commented Aug 31, 2015

LGTM

@bobylito
Copy link
Contributor Author

bobylito commented Sep 1, 2015

Cool :) Mergin'

bobylito added a commit that referenced this pull request Sep 1, 2015
Fix #146 : add a way to make queries that don't change the inner state
@bobylito bobylito merged commit e263e0b into develop Sep 1, 2015
@vvo
Copy link
Contributor

vvo commented Sep 1, 2015

For future PRs, can we agree not to commit the builded files? We should only build when releasing (I guess it was a mistake maybe)

@bobylito
Copy link
Contributor Author

bobylito commented Sep 1, 2015

Yes sure :)

@bobylito bobylito deleted the feature/searchOnce branch September 1, 2015 11:05
dhayab pushed a commit to algolia/instantsearch that referenced this pull request Jul 10, 2023
…eature/searchOnce

Fix algolia/algoliasearch-helper-js#146 : add a way to make queries that don't change the inner state
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.

None yet

3 participants