Skip to content

Commit

Permalink
refactor(history): remove our mode when we handle history object from…
Browse files Browse the repository at this point in the history 
… the history lib (#1540)

- `onStateChange` can now be used without having to provide the `state` prop

BREAKING CHANGE:
- There's no more built in URL synchronisation
- We now provide the semantics and examples to handle URL sychronisation on your side, giving you full power and understanding. Read how to do it here: https://community.algolia.com/wordpress/installation.html
  • Loading branch information
@mthuret @vvo
mthuret authored and vvo committed Nov 14, 2016
1 parent b446cb2 commit ed2ae84
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 583 deletions.
2 changes: 0 additions & 2 deletions packages/react-instantsearch/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,8 @@
"dependencies": {
"algoliasearch": "^3.18.1",
"algoliasearch-helper": "^2.14.0",
"history": "^2.1.2",
"insert-css": "^1.0.0",
"lodash": "^4.16.2",
"qs": "^6.2.1",
"react-addons-shallow-compare": "^15.3.2",
"react-themeable": "^1.1.0"
},
Expand Down
96 changes: 10 additions & 86 deletions packages/react-instantsearch/src/core/InstantSearch.js
View file
Original file line number Diff line number Diff line change
@@ -1,45 +1,19 @@
import {PropTypes, Component, Children} from 'react';
import {createHistory} from 'history';

import createHistoryStateManager from './createHistoryStateManager';
import createInstantSearchManager from './createInstantSearchManager';

function formatProps(props) {
return props.map(prop => `\`${prop}\``).join(', ');
}

const neededProps = ['state', 'onStateChange', 'createURL'];
function validateProps(props) {
const presentProps = [];
const missingProps = [];
for (const prop of neededProps) {
if (props[prop]) {
presentProps.push(prop);
} else {
missingProps.push(prop);
}
}
if (presentProps.length !== 0 && missingProps.length !== 0) {
const missingPlural = missingProps.length > 1;
const presentPlural = presentProps.length > 1;
const missingPropsStr = formatProps(missingProps);
const presentPropsStr = formatProps(presentProps);
const missingPropName = `prop${missingPlural ? 's' : ''}`;
const presentPropName = `prop${presentPlural ? 's' : ''}`;
if (props.state && !props.onStateChange)
throw new Error(
`You must provide ${missingPlural ? '' : 'a '}${missingPropsStr} ` +
`${missingPropName} alongside the ${presentPropsStr} ` +
`${presentPropName} on <InstantSearch>`
'You must provide an `onStateChange` function as a prop if you want to manage the state of <InstantSearch>'
);
}
}

function validateNextProps(props, nextProps) {
if (!props.onStateChange && nextProps.onStateChange) {
if (!props.state && nextProps.state) {
throw new Error(
'You can\'t switch <InstantSearch> from being uncontrolled to controlled'
);
} else if (props.onStateChange && !nextProps.onStateChange) {
} else if (props.state && !nextProps.state) {
throw new Error(
'You can\'t switch <InstantSearch> from being controlled to uncontrolled'
);
Expand Down Expand Up @@ -85,32 +59,9 @@ class InstantSearch extends Component {

validateProps(props);

this.isControlled = Boolean(props.onStateChange);
this.isHSControlled = !this.isControlled &&
(props.history || props.urlSync);
this.isControlled = Boolean(props.state);

let initialState;
if (this.isControlled) {
initialState = props.state;
} else if (this.isHSControlled) {
const hs = props.history || createHistory();
this.hsManager = createHistoryStateManager({
history: hs,
threshold: props.threshold,
onInternalStateUpdate: this.onHistoryInternalStateUpdate.bind(this),
getKnownKeys: this.getKnownKeys.bind(this),
});
// @TODO: Since widgets haven't been registered yet, we have no way of
// knowing which URL query keys are known and which aren't. As such,
// `getStateFromCurrentLocation()` simply returns the current URL query
// deserialized.
// We might want to initialize to an empty state here and call
// `onHistoryInternalStateUpdate` on `componentDidMount`, once all widgets
// have been registered.
initialState = this.hsManager.getStateFromCurrentLocation();
} else {
initialState = {};
}
const initialState = this.isControlled ? props.state : {};

this.aisManager = createInstantSearchManager({
appId: props.appId,
Expand All @@ -131,12 +82,6 @@ class InstantSearch extends Component {
}
}

componentWillUnmount() {
if (this.isHSControlled) {
this.hsManager.unlisten();
}
}

getChildContext() {
// If not already cached, cache the bound methods so that we can forward them as part
// of the context.
Expand All @@ -160,18 +105,7 @@ class InstantSearch extends Component {

createHrefForState(state) {
state = this.aisManager.transitionState(state);

if (this.isControlled) {
return this.props.createURL(state, this.getKnownKeys());
} else if (this.isHSControlled) {
return this.hsManager.createHrefForState(state, this.getKnownKeys());
} else {
return '#';
}
}

onHistoryInternalStateUpdate(state) {
this.aisManager.onExternalStateUpdate(state);
return this.isControlled && this.props.createURL ? this.props.createURL(state, this.getKnownKeys()) : '#';
}

onWidgetsInternalStateUpdate(state) {
Expand All @@ -180,12 +114,10 @@ class InstantSearch extends Component {
if (this.isControlled) {
this.props.onStateChange(state);
} else {
this.aisManager.onExternalStateUpdate(state);
if (this.isHSControlled) {
// This needs to go after the aisManager's update, since it depends on new
// metadata.
this.hsManager.onExternalStateUpdate(state);
if (this.props.onStateChange) {
this.props.onStateChange(state);
}
this.aisManager.onExternalStateUpdate(state);
}
}

Expand All @@ -212,9 +144,6 @@ InstantSearch.propTypes = {

searchParameters: PropTypes.object,

history: PropTypes.object,
urlSync: PropTypes.bool,
threshold: PropTypes.number,
createURL: PropTypes.func,

state: PropTypes.object,
Expand All @@ -223,11 +152,6 @@ InstantSearch.propTypes = {
children: PropTypes.node,
};

InstantSearch.defaultProps = {
urlSync: false,
threshold: 700,
};

InstantSearch.childContextTypes = {
// @TODO: more precise widgets manager propType
ais: PropTypes.object.isRequired,
Expand Down
118 changes: 7 additions & 111 deletions packages/react-instantsearch/src/core/InstantSearch.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,13 @@

import React from 'react';
import {mount} from 'enzyme';
import {createHistory} from 'history';

import InstantSearch from './InstantSearch';

import createHistoryStateManager from './createHistoryStateManager';
jest.mock('./createHistoryStateManager', () => jest.fn(() => ({
getStateFromCurrentLocation: jest.fn(),
})));
import createInstantSearchManager from './createInstantSearchManager';
jest.mock('./createInstantSearchManager', () => jest.fn(() => ({
context: {},
})));
jest.mock('history');

const DEFAULT_PROPS = {
appId: 'foo',
Expand All @@ -26,7 +20,6 @@ const DEFAULT_PROPS = {

describe('InstantSearch', () => {
afterEach(() => {
createHistoryStateManager.mockClear();
createInstantSearchManager.mockClear();
});

Expand Down Expand Up @@ -64,7 +57,7 @@ describe('InstantSearch', () => {
<div />
</InstantSearch>
);
}).toThrowError('You must provide `onStateChange`, `createURL` props alongside the `state` prop on <InstantSearch>');
}).toThrowError('You must provide an `onStateChange` function as a prop if you want to manage the state of <InstantSearch>');

expect(() => {
mount(
Expand All @@ -76,7 +69,7 @@ describe('InstantSearch', () => {
<div />
</InstantSearch>
);
}).toThrowError('You must provide a `onStateChange` prop alongside the `state`, `createURL` props on <InstantSearch>');
}).toThrowError('You must provide an `onStateChange` function as a prop if you want to manage the state of <InstantSearch>');

expect(() => {
const wrapper = mount(
Expand All @@ -92,7 +85,7 @@ describe('InstantSearch', () => {
wrapper.setProps({
state: undefined,
});
}).toThrowError('You must provide a `state` prop alongside the `onStateChange`, `createURL` props on <InstantSearch>');
}).toThrowError('You can\'t switch <InstantSearch> from being controlled to uncontrolled');

expect(() => {
const wrapper = mount(
Expand Down Expand Up @@ -175,46 +168,6 @@ describe('InstantSearch', () => {
});
});

it('works as a history controlled input', () => {
const widgetsIds = [];
const ism = {
transitionState: state => ({...state, transitioned: true}),
onExternalStateUpdate: jest.fn(),
getWidgetsIds: () => widgetsIds,
};
createInstantSearchManager.mockImplementation(() => ism);
const initialState = {a: 0};
const hsm = {
getStateFromCurrentLocation: jest.fn(() => initialState),
onExternalStateUpdate: jest.fn(),
createHrefForState: jest.fn(state => state),
};
createHistoryStateManager.mockImplementation(() => hsm);

const history = {};
const threshold = 666;
const wrapper = mount(
<InstantSearch {...DEFAULT_PROPS} history={history} threshold={threshold}>
<div />
</InstantSearch>
);
const nextState = {a: 1};

const args = createHistoryStateManager.mock.calls[0][0];
expect(args.history).toBe(history);
expect(args.threshold).toBe(threshold);
expect(args.getKnownKeys()).toBe(widgetsIds);
args.onInternalStateUpdate(nextState);
expect(ism.onExternalStateUpdate.mock.calls[0][0]).toBe(nextState);

const {ais: {onInternalStateUpdate}} = wrapper.instance().getChildContext();
onInternalStateUpdate(nextState);
expect(hsm.onExternalStateUpdate.mock.calls[0][0]).toEqual({
a: 1,
transitioned: true,
});
});

it('works as an uncontrolled input', () => {
const ism = {
transitionState: state => ({...state, transitioned: true}),
Expand All @@ -235,19 +188,11 @@ describe('InstantSearch', () => {
a: 1,
transitioned: true,
});
});

it('creates its own history if urlSync=true and history=undefined', () => {
const history = {};
createHistory.mockImplementationOnce(() => history);
mount(
<InstantSearch {...DEFAULT_PROPS} urlSync={true}>
<div />
</InstantSearch>
);

const args = createHistoryStateManager.mock.calls[0][0];
expect(args.history).toBe(history);
const onStateChange = jest.fn();
wrapper.setProps({onStateChange});
onInternalStateUpdate({a: 2});
expect(onStateChange.mock.calls[0][0]).toEqual({a: 2, transitioned: true});
});

it('exposes the isManager\'s store and widgetsManager in context', () => {
Expand Down Expand Up @@ -294,36 +239,6 @@ describe('InstantSearch', () => {
expect(createURL.mock.calls[0][1]).toBe(widgetsIds);
});

it('passes through to the hsManager when it is defined', () => {
const widgetsIds = [];
const ism = {
transitionState: state => ({...state, transitioned: true}),
getWidgetsIds: () => widgetsIds,
};
createInstantSearchManager.mockImplementation(() => ism);
const initialState = {a: 0};
const hsm = {
getStateFromCurrentLocation: jest.fn(() => initialState),
onExternalStateUpdate: jest.fn(),
createHrefForState: jest.fn(state => state),
};
createHistoryStateManager.mockImplementation(() => hsm);

const wrapper = mount(
<InstantSearch
{...DEFAULT_PROPS}
urlSync
>
<div />
</InstantSearch>
);

const {ais: {createHrefForState}} = wrapper.instance().getChildContext();
const outputURL = createHrefForState({a: 1});
expect(outputURL).toEqual({a: 1, transitioned: true});
expect(hsm.createHrefForState.mock.calls[0][1]).toBe(widgetsIds);
});

it('returns # otherwise', () => {
const wrapper = mount(
<InstantSearch {...DEFAULT_PROPS}>
Expand All @@ -336,23 +251,4 @@ describe('InstantSearch', () => {
expect(outputURL).toBe('#');
});
});

it('cleans after itself when it unmounts', () => {
const hsm = {
getStateFromCurrentLocation: jest.fn(() => ({})),
unlisten: jest.fn(),
};
createHistoryStateManager.mockImplementation(() => hsm);

const wrapper = mount(
<InstantSearch
{...DEFAULT_PROPS}
urlSync
>
<div />
</InstantSearch>
);
wrapper.unmount();
expect(hsm.unlisten.mock.calls.length).toBe(1);
});
});
Loading

0 comments on commit ed2ae84

Please sign in to comment.