Skip to content

Commit

Permalink
Merge branch 'feat/send-events' into feat/send-events-2
Browse files Browse the repository at this point in the history
  • Loading branch information
eunjae-lee committed Aug 27, 2020
2 parents ce4f5f0 + 02781f7 commit ef2d7df
Show file tree
Hide file tree
Showing 11 changed files with 245 additions and 217 deletions.
32 changes: 31 additions & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,21 @@ jobs:
command: |
yarn shipjs trigger
'prepare release':
<<: *defaults
steps:
- checkout
- run: *install_yarn_version
- restore_cache: *restore_yarn_cache
- run: *run_yarn_install
- save_cache: *save_yarn_cache
- run:
name: Prepare a pull request for next release
command: |
git config --global user.email "[email protected]"
git config --global user.name "InstantSearch"
yarn run release --yes --no-browse
workflows:
version: 2
ci:
Expand All @@ -134,4 +149,19 @@ workflows:
- type check algoliasearch v3
- type check js (optional)
- e2e tests
- release if needed
- 'release if needed':
filters:
branches:
only:
- master
- next
'scheduled release':
triggers:
- schedule:
cron: '0 9 * * 2'
filters:
branches:
only:
- master
jobs:
- prepare release
8 changes: 7 additions & 1 deletion babel.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,13 @@ module.exports = api => {
const buildPlugins = clean([
'@babel/plugin-proposal-class-properties',
'@babel/plugin-transform-react-constant-elements',
'babel-plugin-transform-react-remove-prop-types',
[
'babel-plugin-transform-react-remove-prop-types',
{
mode: 'remove',
removeImport: true,
},
],
'babel-plugin-transform-react-pure-class-to-function',
wrapWarningWithDevCheck,
(isCJS || isES) && [
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@
"scriptjs": "2.5.9",
"semver": "6.3.0",
"shelljs": "0.8.3",
"shipjs": "0.16.0",
"shipjs": "0.21.0",
"typescript": "3.8.3",
"webpack": "4.41.5"
},
Expand Down
7 changes: 2 additions & 5 deletions ship.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ const fs = require('fs');
const path = require('path');

module.exports = {
mergeStrategy: { toSameBranch: ['master', 'next'] },
shouldPrepare: ({ releaseType, commitNumbersPerType }) => {
const { fix = 0 } = commitNumbersPerType;
if (releaseType === 'patch' && fix === 0) {
Expand All @@ -20,10 +19,9 @@ module.exports = {
beforeCommitChanges: ({ exec }) => {
exec('yarn doctoc');
},
pullRequestTeamReviewer: ['instantsearch-for-websites'],
pullRequestTeamReviewers: ['instantsearch-for-websites'],
buildCommand: ({ version }) =>
`NODE_ENV=production VERSION=${version} yarn build`,
testCommandBeforeRelease: () => 'echo "No need to test again."',
afterPublish: ({ exec, version, releaseTag }) => {
if (releaseTag === 'latest' && version.startsWith('4.')) {
exec('./scripts/release/build-experimental-typescript.js');
Expand All @@ -33,10 +31,9 @@ module.exports = {
}
},
slack: {
// disable slack notification for `prepared` and `releaseStart` lifecycle.
// disable slack notification for `prepared` lifecycle.
// Ship.js will send slack message only for `releaseSuccess`.
prepared: null,
releaseStart: null,
releaseSuccess: ({
appName,
version,
Expand Down
97 changes: 85 additions & 12 deletions src/lib/__tests__/RoutingManager-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -313,12 +313,9 @@ describe('RoutingManager', () => {

await runAllMicroTasks();

expect(router.write).toHaveBeenCalledTimes(2);
expect(router.write).toHaveBeenLastCalledWith({
indexName: {
query: 'Apple',
},
});
// The UI state hasn't changed so `router.write` wasn't called a second
// time
expect(router.write).toHaveBeenCalledTimes(1);
});

test('should keep the UI state up to date on first render', async () => {
Expand Down Expand Up @@ -363,12 +360,9 @@ describe('RoutingManager', () => {

await runAllMicroTasks();

expect(router.write).toHaveBeenCalledTimes(2);
expect(router.write).toHaveBeenLastCalledWith({
indexName: {
query: 'Apple iPhone',
},
});
// The UI state hasn't changed so `router.write` wasn't called a second
// time
expect(router.write).toHaveBeenCalledTimes(1);
});

test('should keep the UI state up to date on router.update', async () => {
Expand Down Expand Up @@ -443,6 +437,85 @@ describe('RoutingManager', () => {
},
});
});

test('skips duplicate route state entries', async () => {
let triggerChange = false;
const searchClient = createSearchClient();
const stateMapping = createFakeStateMapping({
stateToRoute(uiState) {
if (triggerChange) {
return {
...uiState,
indexName: {
...uiState.indexName,
triggerChange,
},
};
}

return uiState;
},
});
const history = createFakeHistory();
const router = createFakeRouter({
onUpdate(fn) {
history.subscribe(state => {
fn(state);
});
},
write: jest.fn(state => {
history.push(state);
}),
});

const search = instantsearch({
indexName: 'indexName',
searchClient,
routing: {
router,
stateMapping,
},
});

const fakeSearchBox: any = createFakeSearchBox();
const fakeHitsPerPage1 = createFakeHitsPerPage();
const fakeHitsPerPage2 = createFakeHitsPerPage();

search.addWidgets([fakeSearchBox, fakeHitsPerPage1, fakeHitsPerPage2]);

search.start();

await runAllMicroTasks();

// Trigger an update - push a change
fakeSearchBox.refine('Apple');

expect(router.write).toHaveBeenCalledTimes(1);
expect(router.write).toHaveBeenLastCalledWith({
indexName: {
query: 'Apple',
},
});

// Trigger change without UI state change
search.removeWidgets([fakeHitsPerPage1]);

expect(router.write).toHaveBeenCalledTimes(1);

await runAllMicroTasks();

triggerChange = true;
// Trigger change without UI state change but with a route change
search.removeWidgets([fakeHitsPerPage2]);

expect(router.write).toHaveBeenCalledTimes(2);
expect(router.write).toHaveBeenLastCalledWith({
indexName: {
query: 'Apple',
triggerChange: true,
},
});
});
});

describe('windowTitle', () => {
Expand Down
105 changes: 0 additions & 105 deletions src/lib/routers/__tests__/history.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,111 +8,6 @@ describe('life cycle', () => {
jest.restoreAllMocks();
});

it('does not write the same url twice', async () => {
const pushState = jest.spyOn(window.history, 'pushState');
const router = historyRouter({
writeDelay: 0,
});

router.write({ some: 'state' });
await wait(0);

router.write({ some: 'state' });
await wait(0);

router.write({ some: 'state' });
await wait(0);

expect(pushState).toHaveBeenCalledTimes(1);
expect(pushState.mock.calls).toMatchInlineSnapshot(`
Array [
Array [
Object {
"some": "state",
},
"",
"http://localhost/?some=state",
],
]
`);
});

it('does not write if already externally updated to desired URL', async () => {
const pushState = jest.spyOn(window.history, 'pushState');
const router = historyRouter({
writeDelay: 0,
});

const fakeState = { identifier: 'fake state' };

router.write({ some: 'state one' });

// external update before timeout passes
window.history.pushState(
fakeState,
'',
'http://localhost/?some=state%20two'
);

// this write isn't needed anymore
router.write({ some: 'state two' });
await wait(0);

expect(pushState).toHaveBeenCalledTimes(1);
expect(pushState.mock.calls).toMatchInlineSnapshot(`
Array [
Array [
Object {
"identifier": "fake state",
},
"",
"http://localhost/?some=state%20two",
],
]
`);

// proves that InstantSearch' write did not happen
expect(history.state).toBe(fakeState);
});

it('does not write the same url title twice', async () => {
const title = jest.spyOn(window.document, 'title', 'set');
const pushState = jest.spyOn(window.history, 'pushState');

const router = historyRouter({
writeDelay: 0,
windowTitle: state => `My Site - ${state.some}`,
});

expect(title).toHaveBeenCalledTimes(1);
expect(window.document.title).toBe('My Site - undefined');

router.write({ some: 'state' });
await wait(0);

router.write({ some: 'state' });
await wait(0);

router.write({ some: 'state' });
await wait(0);

expect(pushState).toHaveBeenCalledTimes(1);
expect(pushState.mock.calls).toMatchInlineSnapshot(`
Array [
Array [
Object {
"some": "state",
},
"My Site - state",
"http://localhost/?some=state",
],
]
`);

expect(title).toHaveBeenCalledTimes(2);
expect(window.document.title).toBe('My Site - state');
});

it('writes after timeout is done', async () => {
const pushState = jest.spyOn(window.history, 'pushState');

Expand Down
7 changes: 2 additions & 5 deletions src/lib/routers/history.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,8 @@ class BrowserHistory implements Router {
}

this.writeTimer = window.setTimeout(() => {
if (window.location.href !== url) {
setWindowTitle(title);

window.history.pushState(routeState, title || '', url);
}
setWindowTitle(title);
window.history.pushState(routeState, title || '', url);
this.writeTimer = undefined;
}, this.writeDelay);
}
Expand Down
23 changes: 19 additions & 4 deletions src/middlewares/createRouter.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
import simpleStateMapping from '../lib/stateMappings/simple';
import historyRouter from '../lib/routers/history';
import { Index } from '../widgets/index/index';
import { Router, StateMapping, UiState, Middleware } from '../types';
import {
Router,
StateMapping,
UiState,
Middleware,
RouteState,
} from '../types';
import { isEqual } from '../lib/utils';

const walk = (current: Index, callback: (index: Index) => void) => {
callback(current);
Expand Down Expand Up @@ -49,11 +56,19 @@ export const createRouter: RoutingManager = (props = {}) => {
...stateMapping.routeToState(router.read()),
};

let lastRouteState: RouteState | undefined = undefined;

return {
onStateChange({ uiState }) {
const route = stateMapping.stateToRoute(uiState);

router.write(route);
const routeState = stateMapping.stateToRoute(uiState);

if (
lastRouteState === undefined ||
!isEqual(lastRouteState, routeState)
) {
router.write(routeState);
lastRouteState = routeState;
}
},

subscribe() {
Expand Down
Loading

0 comments on commit ef2d7df

Please sign in to comment.