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

UI: Migrate router to react-router #16440

Merged
merged 14 commits into from
Oct 25, 2021
Merged
1 change: 1 addition & 0 deletions examples/official-storybook/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"@storybook/jest": "0.0.0-alpha.5",
"@storybook/node-logger": "6.4.0-beta.19",
"@storybook/react": "6.4.0-beta.19",
"@storybook/router": "6.4.0-beta.19",
"@storybook/source-loader": "6.4.0-beta.19",
"@storybook/testing-library": "0.0.0-alpha.3",
"@storybook/theming": "6.4.0-beta.19",
Expand Down
3 changes: 0 additions & 3 deletions lib/api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,21 +38,18 @@
"prepare": "node ../../scripts/prepare.js"
},
"dependencies": {
"@reach/router": "^1.3.4",
"@storybook/channels": "6.4.0-beta.19",
"@storybook/client-logger": "6.4.0-beta.19",
"@storybook/core-events": "6.4.0-beta.19",
"@storybook/csf": "0.0.2--canary.87bc651.0",
"@storybook/router": "6.4.0-beta.19",
"@storybook/semver": "^7.3.2",
"@storybook/theming": "6.4.0-beta.19",
"@types/reach__router": "^1.3.7",
"core-js": "^3.8.2",
"fast-deep-equal": "^3.1.3",
"global": "^4.4.0",
"lodash": "^4.17.20",
"memoizerific": "^1.11.3",
"qs": "^6.10.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

tests are still using it, so it should stay in devDependencies

Copy link
Member

Choose a reason for hiding this comment

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

Any chance you can make a PR? 🙏

Copy link
Contributor

Choose a reason for hiding this comment

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

@shilman here you go: #16525

"regenerator-runtime": "^0.13.7",
"store2": "^2.12.0",
"telejson": "^5.3.2",
Expand Down
2 changes: 1 addition & 1 deletion lib/api/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
SHARED_STATE_SET,
SET_STORIES,
} from '@storybook/core-events';
import { RenderData as RouterData } from '@storybook/router';
import { RouterData } from '@storybook/router';
import { Listener } from '@storybook/channels';

import { createContext } from './context';
Expand Down
6 changes: 3 additions & 3 deletions lib/api/src/modules/addons.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { ReactElement } from 'react';
import { WindowLocation } from '@reach/router';
import type { RenderData } from '@storybook/router';
import deprecate from 'util-deprecate';
import dedent from 'ts-dedent';

Expand Down Expand Up @@ -35,13 +35,13 @@ export interface RenderOptions {
export interface RouteOptions {
storyId: string;
viewMode: ViewMode;
location: WindowLocation;
location: RenderData['location'];
path: string;
}
export interface MatchOptions {
storyId: string;
viewMode: ViewMode;
location: WindowLocation;
location: RenderData['location'];
path: string;
}

Expand Down
27 changes: 13 additions & 14 deletions lib/api/src/modules/url.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import { navigate as navigateRouter, NavigateOptions } from '@reach/router';
import { once } from '@storybook/client-logger';
import {
NAVIGATE_URL,
STORY_ARGS_UPDATED,
SET_CURRENT_STORY,
GLOBALS_UPDATED,
} from '@storybook/core-events';
import { queryFromLocation, navigate as queryNavigate, buildArgsParam } from '@storybook/router';
import { queryFromLocation, buildArgsParam, NavigateOptions } from '@storybook/router';
import { toId, sanitize } from '@storybook/csf';
import deepEqual from 'fast-deep-equal';
import global from 'global';
Expand All @@ -28,15 +27,6 @@ const parseBoolean = (value: string) => {
return undefined;
};

const navigateTo = (path: string, queryParams: Record<string, string> = {}, options = {}) => {
const params = Object.entries(queryParams)
.filter(([, v]) => v)
.sort(([a], [b]) => (a < b ? -1 : 1))
.map(([k, v]) => `${k}=${v}`);
const to = [path, ...params].join('&');
return queryNavigate(to, options);
};

// Initialize the state based on the URL.
// NOTE:
// Although we don't change the URL when you change the state, we do support setting initial state
Expand Down Expand Up @@ -130,7 +120,7 @@ export interface QueryParams {
}

export interface SubAPI {
navigateUrl: (url: string, options: NavigateOptions<{}>) => void;
navigateUrl: (url: string, options: NavigateOptions) => void;
getQueryParam: (key: string) => string | undefined;
getUrlState: () => {
queryParams: QueryParams;
Expand All @@ -143,6 +133,15 @@ export interface SubAPI {
}

export const init: ModuleFn = ({ store, navigate, state, provider, fullAPI, ...rest }) => {
const navigateTo = (path: string, queryParams: Record<string, string> = {}, options = {}) => {
const params = Object.entries(queryParams)
.filter(([, v]) => v)
.sort(([a], [b]) => (a < b ? -1 : 1))
.map(([k, v]) => `${k}=${v}`);
const to = [path, ...params].join('&');
return navigate(to, options);
};

const api: SubAPI = {
getQueryParam(key) {
const { customQueryParams } = store.getState();
Expand All @@ -167,8 +166,8 @@ export const init: ModuleFn = ({ store, navigate, state, provider, fullAPI, ...r
const equal = deepEqual(customQueryParams, update);
if (!equal) store.setState({ customQueryParams: update });
},
navigateUrl(url: string, options: NavigateOptions<{}>) {
navigateRouter(url, options);
navigateUrl(url, options) {
navigate(url, { ...options, plain: true });
},
};

Expand Down
29 changes: 16 additions & 13 deletions lib/api/src/tests/url.test.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
import qs from 'qs';

import { SET_CURRENT_STORY, GLOBALS_UPDATED } from '@storybook/core-events';
import { navigate as reachNavigate } from '@reach/router';

import { init as initURL } from '../modules/url';

jest.mock('@storybook/client-logger');
jest.mock('@reach/router');
jest.useFakeTimers();

describe('initial state', () => {
Expand Down Expand Up @@ -193,7 +191,9 @@ describe('initModule', () => {
it('updates args param on SET_CURRENT_STORY', async () => {
store.setState(storyState('test--story'));

const { api, init } = initURL({ store, state: { location: {} }, fullAPI });
const navigate = jest.fn();

const { api, init } = initURL({ store, state: { location: {} }, navigate, fullAPI });
Object.assign(fullAPI, api, {
getCurrentStoryData: () => ({
args: { a: 1, b: 2 },
Expand All @@ -204,8 +204,8 @@ describe('initModule', () => {
init();

fullAPI.emit(SET_CURRENT_STORY);
expect(reachNavigate).toHaveBeenCalledWith(
'/?path=/story/test--story&args=b:2',
expect(navigate).toHaveBeenCalledWith(
'/story/test--story&args=b:2',
expect.objectContaining({ replace: true })
);
expect(store.getState().customQueryParams).toEqual({ args: 'b:2' });
Expand All @@ -214,36 +214,39 @@ describe('initModule', () => {
it('updates globals param on GLOBALS_UPDATED', async () => {
store.setState(storyState('test--story'));

const { api, init } = initURL({ store, state: { location: {} }, fullAPI });
const navigate = jest.fn();

const { api, init } = initURL({ store, state: { location: {} }, navigate, fullAPI });
Object.assign(fullAPI, api);
init();

fullAPI.emit(GLOBALS_UPDATED, { globals: { a: 2 }, initialGlobals: { a: 1, b: 1 } });
expect(reachNavigate).toHaveBeenCalledWith(
'/?path=/story/test--story&globals=a:2;b:!undefined',
expect(navigate).toHaveBeenCalledWith(
'/story/test--story&globals=a:2;b:!undefined',
expect.objectContaining({ replace: true })
);
expect(store.getState().customQueryParams).toEqual({ globals: 'a:2;b:!undefined' });
});

it('adds url params alphabetically', async () => {
store.setState({ ...storyState('test--story'), customQueryParams: { full: 1 } });
const navigate = jest.fn();

const { api, init } = initURL({ store, state: { location: {} }, fullAPI });
const { api, init } = initURL({ store, state: { location: {} }, navigate, fullAPI });
Object.assign(fullAPI, api, {
getCurrentStoryData: () => ({ args: { a: 1 }, isLeaf: true }),
});
init();

fullAPI.emit(GLOBALS_UPDATED, { globals: { g: 2 } });
expect(reachNavigate).toHaveBeenCalledWith(
'/?path=/story/test--story&full=1&globals=g:2',
expect(navigate).toHaveBeenCalledWith(
'/story/test--story&full=1&globals=g:2',
expect.objectContaining({ replace: true })
);

fullAPI.emit(SET_CURRENT_STORY);
expect(reachNavigate).toHaveBeenCalledWith(
'/?path=/story/test--story&args=a:1&full=1&globals=g:2',
expect(navigate).toHaveBeenCalledWith(
'/story/test--story&args=a:1&full=1&globals=g:2',
expect.objectContaining({ replace: true })
);
});
Expand Down
13 changes: 8 additions & 5 deletions lib/core-common/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,10 +261,13 @@ export type Preset =
*/
export type Entry = string;

type StorybookRefs = Record<string, {
title: string;
url: string;
}>;
type StorybookRefs = Record<
string,
{
title: string;
url: string;
}
>;

/**
* The interface for Storybook configuration in `main.ts` files.
Expand Down Expand Up @@ -337,7 +340,7 @@ export interface StorybookConfig {
/**
* References external Storybooks
*/
refs?: StorybookRefs | ((config: Configuration, options: Options) => StorybookRefs)
refs?: StorybookRefs | ((config: Configuration, options: Options) => StorybookRefs);

/**
* Modify or return a custom Webpack config.
Expand Down
2 changes: 1 addition & 1 deletion lib/router/README.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Storybook Router

Storybook Router is a wrapper library for reach/router.
Storybook Router is a wrapper library for react-router.
It ensures a single version of the router is used everywhere.
It also includes some ready to use utils to read the path, query, viewMode and storyId from location.
5 changes: 3 additions & 2 deletions lib/router/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,16 @@
"prepare": "node ../../scripts/prepare.js"
},
"dependencies": {
"@reach/router": "^1.3.4",
"@storybook/client-logger": "6.4.0-beta.19",
"@types/reach__router": "^1.3.7",
"core-js": "^3.8.2",
"fast-deep-equal": "^3.1.3",
"global": "^4.4.0",
"history": "^5.0.1",
"lodash": "^4.17.20",
"memoizerific": "^1.11.3",
"qs": "^6.10.0",
"react-router": "^6.0.0-beta.7",
"react-router-dom": "^6.0.0-beta.7",
"ts-dedent": "^2.0.0"
},
"peerDependencies": {
Expand Down
78 changes: 50 additions & 28 deletions lib/router/src/router.tsx
Original file line number Diff line number Diff line change
@@ -1,17 +1,14 @@
import global from 'global';
import React, { ReactNode } from 'react';
import React, { ReactNode, useCallback } from 'react';

import {
Link,
Location,
navigate,
LocationProvider,
RouteComponentProps,
LocationContext,
NavigateFn,
BrowserRouter,
useNavigate,
useLocation,
NavigateOptions,
History,
} from '@reach/router';
Router,
} from 'react-router-dom';
import { ToggleVisibility } from './visibility';
import { queryFromString, parsePath, getMatch, StoryData } from './utils';

Expand All @@ -22,9 +19,12 @@ interface Other extends StoryData {
singleStory?: boolean;
}

export type RenderData = Pick<LocationContext, 'location'> &
Partial<Pick<LocationContext, 'navigate'>> &
Other;
export type RouterData = {
location: Partial<Location>;
navigate: ReturnType<typeof useQueryNavigate>;
} & Other;

export type RenderData = Pick<RouterData, 'location'> & Other;

interface MatchingData {
match: null | { path: string };
Expand Down Expand Up @@ -52,8 +52,26 @@ export interface QueryLinkProps {

const getBase = () => `${document.location.pathname}?`;

const queryNavigate: NavigateFn = (to: string | number, options?: NavigateOptions<{}>) =>
typeof to === 'number' ? navigate(to) : navigate(`${getBase()}path=${to}`, options);
type ExpandedNavigateOptions = NavigateOptions & { plain?: boolean };

// const queryNavigate: NavigateFn = (to: string | number, options?: NavigateOptions<{}>) =>
// typeof to === 'number' ? navigate(to) : navigate(`${getBase()}path=${to}`, options);

const useQueryNavigate = () => {
const navigate = useNavigate();

return useCallback((to: string | number, options?: ExpandedNavigateOptions) => {
if (typeof to === 'string') {
const target = options?.plain ? to : `?path=${to}`;
return navigate(target, options);
}
if (typeof to === 'number') {
return navigate(to);
}

return undefined;
}, []);
};

// A component that will navigate to a new location/path when clicked
const QueryLink = ({ to, children, ...rest }: QueryLinkProps) => (
Expand All @@ -65,24 +83,24 @@ QueryLink.displayName = 'QueryLink';

// A render-prop component where children is called with a location
// and will be called whenever it changes when it changes
const QueryLocation = ({ children }: QueryLocationProps) => (
<Location>
{({ location }: RouteComponentProps): ReactNode => {
const { path, singleStory } = queryFromString(location.search);
const { viewMode, storyId, refId } = parsePath(path);
const QueryLocation = ({ children }: QueryLocationProps) => {
const location = useLocation();
const { path, singleStory } = queryFromString(location.search);
const { viewMode, storyId, refId } = parsePath(path);

return children({
return (
<>
{children({
path,
location,
navigate: queryNavigate,
viewMode,
storyId,
refId,
singleStory: singleStory === 'true',
});
}}
</Location>
);
})}
</>
);
};
QueryLocation.displayName = 'QueryLocation';

// A render-prop component for rendering when a certain path is hit.
Expand Down Expand Up @@ -117,6 +135,10 @@ export { QueryLink as Link };
export { QueryMatch as Match };
export { QueryLocation as Location };
export { Route };
export { queryNavigate as navigate };
export { LocationProvider };
export type { History };
export { useQueryNavigate as useNavigate };
export { BrowserRouter as LocationProvider };
export { Router as BaseLocationProvider };
export { useNavigate as usePlainNavigate };

// eslint-disable-next-line no-undef
export type { ExpandedNavigateOptions as NavigateOptions };
2 changes: 1 addition & 1 deletion lib/router/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ interface Query {
export const queryFromString = memoize(1000)(
(s: string): Query => qs.parse(s, { ignoreQueryPrefix: true })
);
export const queryFromLocation = (location: { search: string }) => queryFromString(location.search);
export const queryFromLocation = (location: Partial<Location>) => queryFromString(location.search);
export const stringifyQuery = (query: Query) =>
qs.stringify(query, { addQueryPrefix: true, encode: false });

Expand Down
Loading