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

Core: Support null and undefined in URL args #14049

Merged
merged 8 commits into from
Feb 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion lib/client-api/src/args.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { mapArgsToTypes } from './args';
import { combineArgs, mapArgsToTypes } from './args';

const stringType = { name: 'string' };
const numberType = { name: 'number' };
Expand Down Expand Up @@ -97,3 +97,23 @@ describe('mapArgsToTypes', () => {
});
});
});

describe('combineArgs', () => {
it('merges args', () => {
expect(combineArgs({ foo: 1 }, { bar: 2 })).toStrictEqual({ foo: 1, bar: 2 });
});

it('replaces arrays', () => {
expect(combineArgs({ foo: [1, 2] }, { foo: [3] })).toStrictEqual({ foo: [3] });
});

it('deeply merges args', () => {
expect(combineArgs({ foo: { bar: [1, 2], baz: true } }, { foo: { bar: [3] } })).toStrictEqual({
foo: { bar: [3], baz: true },
});
});

it('omits keys with undefined value', () => {
expect(combineArgs({ foo: 1 }, { foo: undefined })).toStrictEqual({});
});
});
31 changes: 24 additions & 7 deletions lib/client-api/src/args.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import { Args, ArgTypes } from '@storybook/addons';
import { isPlainObject } from 'lodash';

type ValueType = { name: string; value?: ObjectValueType | ValueType };
type ObjectValueType = Record<string, ValueType>;

const map = (arg: any, type: ValueType): any => {
const INCOMPATIBLE = Symbol('incompatible');
const map = (arg: unknown, type: ValueType): any => {
if (arg === undefined || arg === null) return arg;
switch (type?.name) {
case 'string':
return String(arg);
Expand All @@ -12,24 +15,38 @@ const map = (arg: any, type: ValueType): any => {
case 'boolean':
return arg === 'true';
case 'array':
if (!type.value || !Array.isArray(arg)) return undefined;
if (!type.value || !Array.isArray(arg)) return INCOMPATIBLE;
return arg.reduce((acc, item) => {
const mapped = map(item, type.value as ValueType);
return mapped === undefined ? acc : acc.concat([mapped]);
return mapped === INCOMPATIBLE ? acc : acc.concat([mapped]);
}, []);
case 'object':
if (!type.value || typeof arg !== 'object') return undefined;
if (!type.value || typeof arg !== 'object') return INCOMPATIBLE;
return Object.entries(arg).reduce((acc, [key, val]) => {
const mapped = map(val, (type.value as ObjectValueType)[key]);
return mapped === undefined ? acc : Object.assign(acc, { [key]: mapped });
return mapped === INCOMPATIBLE ? acc : Object.assign(acc, { [key]: mapped });
}, {} as Args);
default:
return undefined;
return INCOMPATIBLE;
}
};

export const mapArgsToTypes = (args: Args, argTypes: ArgTypes): Args => {
return Object.entries(args).reduce((acc, [key, value]) => {
return Object.assign(acc, { [key]: map(value, argTypes[key]?.type) });
const mapped = map(value, argTypes[key]?.type);
return mapped === INCOMPATIBLE ? acc : Object.assign(acc, { [key]: mapped });
}, {});
};

export const combineArgs = (value: any, update: any): Args => {
if (!isPlainObject(value) || !isPlainObject(update)) return update;
return Object.keys({ ...value, ...update }).reduce((acc, key) => {
if (key in update) {
const combined = combineArgs(value[key], update[key]);
if (combined !== undefined) acc[key] = combined;
} else {
acc[key] = value[key];
}
return acc;
}, {} as any);
};
4 changes: 2 additions & 2 deletions lib/client-api/src/story_store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import {
StoreSelectionSpecifier,
StoreSelection,
} from './types';
import { mapArgsToTypes } from './args';
import { combineArgs, mapArgsToTypes } from './args';
import { HooksContext } from './hooks';
import { storySort } from './storySort';
import { combineParameters } from './parameters';
Expand Down Expand Up @@ -241,7 +241,7 @@ export default class StoryStore {
if (foundStory) {
if (args && foundStory.args) {
const mappedUrlArgs = mapArgsToTypes(args, foundStory.argTypes);
foundStory.args = combineParameters(foundStory.args, mappedUrlArgs);
foundStory.args = combineArgs(foundStory.args, mappedUrlArgs);
}
this.setSelection({ storyId: foundStory.id, viewMode });
this._channel.emit(Events.STORY_SPECIFIED, { storyId: foundStory.id, viewMode });
Expand Down
1 change: 1 addition & 0 deletions lib/core-client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
"ansi-to-html": "^0.6.11",
"core-js": "^3.8.2",
"global": "^4.4.0",
"lodash": "^4.17.20",
"qs": "^6.9.5",
"regenerator-runtime": "^0.13.7",
"ts-dedent": "^2.0.0",
Expand Down
16 changes: 13 additions & 3 deletions lib/core-client/src/preview/parseArgsParam.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,19 @@ describe('parseArgsParam', () => {
expect(args).toEqual({ key: 'val' });
});

it('parses a space', () => {
const args = parseArgsParam('key:one+two');
expect(args).toEqual({ key: 'one two' });
it('parses spaces', () => {
const args = parseArgsParam('key:one+two+three');
expect(args).toEqual({ key: 'one two three' });
});

it('parses null', () => {
const args = parseArgsParam('key:!null');
expect(args).toEqual({ key: null });
});

it('parses undefined', () => {
const args = parseArgsParam('key:!undefined');
expect(args).toEqual({ key: undefined });
});

it('parses multiple values', () => {
Expand Down
20 changes: 17 additions & 3 deletions lib/core-client/src/preview/parseArgsParam.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,34 @@
import qs from 'qs';
import { Args } from '@storybook/addons';
import { once } from '@storybook/client-logger';
import isPlainObject from 'lodash/isPlainObject';

// Keep this in sync with validateArgs in @storybook/core
const VALIDATION_REGEXP = /^[a-zA-Z0-9 _-]*$/;
const validateArgs = (key = '', value: any = ''): boolean => {
if (key === null || value === null) return false;
const validateArgs = (key = '', value: unknown): boolean => {
if (value === null || value === undefined) return true; // encoded as `!null` or `!undefined`
if (key === null) return false;
if (key === '' || !VALIDATION_REGEXP.test(key)) return false;
if (typeof value === 'number' || typeof value === 'boolean') return true;
if (typeof value === 'string') return VALIDATION_REGEXP.test(value);
if (Array.isArray(value)) return value.every((v) => validateArgs(key, v));
return Object.entries(value).every(([k, v]) => validateArgs(k, v));
if (isPlainObject(value)) return Object.entries(value).every(([k, v]) => validateArgs(k, v));
return false;
ghengeveld marked this conversation as resolved.
Show resolved Hide resolved
};

const QS_OPTIONS = {
delimiter: ';', // we're parsing a single query param
allowDots: true, // objects are encoded using dot notation
decoder(
str: string,
defaultDecoder: (str: string, decoder?: any, charset?: string) => string,
charset: string,
type: 'key' | 'value'
) {
if (type === 'value' && str === '!undefined') return undefined;
if (type === 'value' && str === '!null') return null;
return defaultDecoder(str, defaultDecoder, charset);
},
};
export const parseArgsParam = (argsString: string): Args => {
const parts = argsString.split(';').map((part) => part.replace('=', '~').replace(':', '='));
Expand Down
1 change: 1 addition & 0 deletions lib/router/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,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.9.5"
},
Expand Down
63 changes: 62 additions & 1 deletion lib/router/src/utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { buildArgsParam, getMatch, parsePath } from './utils';
import { buildArgsParam, deepDiff, DEEPLY_EQUAL, getMatch, parsePath } from './utils';

jest.mock('@storybook/client-logger', () => ({
once: { warn: jest.fn() },
Expand Down Expand Up @@ -76,6 +76,42 @@ describe('parsePath', () => {
});
});

describe('deepDiff', () => {
it('returns DEEPLY_EQUAL when the values are deeply equal', () => {
expect(deepDiff({ foo: [{ bar: 1 }] }, { foo: [{ bar: 1 }] })).toBe(DEEPLY_EQUAL);
});

it('returns the update when the types are different', () => {
expect(deepDiff(true, 1)).toBe(1);
});

it('returns the full array when updating an array', () => {
expect(deepDiff([1, 2], [3, 4])).toStrictEqual([3, 4]);
});

it('returns undefined for omitted array values', () => {
expect(deepDiff([1, 2], [3])).toStrictEqual([3, undefined]);
});

it('returns a longer array when adding to an array', () => {
expect(deepDiff([1, 2], [3, 4, 5])).toStrictEqual([3, 4, 5]);
});

it('returns a partial when updating an object', () => {
expect(deepDiff({ foo: 1, bar: 2 }, { foo: 1, bar: 3 })).toStrictEqual({ bar: 3 });
});

it('returns undefined for omitted object properties', () => {
expect(deepDiff({ foo: 1, bar: 2 }, { foo: 1 })).toStrictEqual({ bar: undefined });
});

it('traverses into objects', () => {
expect(deepDiff({ foo: { bar: [1, 2], baz: [3, 4] } }, { foo: { bar: [3] } })).toStrictEqual({
foo: { bar: [3, undefined], baz: undefined },
});
});
});

describe('buildArgsParam', () => {
it('builds a simple key-value pair', () => {
const param = buildArgsParam({}, { key: 'val' });
Expand Down Expand Up @@ -127,6 +163,21 @@ describe('buildArgsParam', () => {
expect(param).toEqual('arr[0].foo.bar:val');
});

it('encodes space as +', () => {
const param = buildArgsParam({}, { key: 'foo bar baz' });
expect(param).toEqual('key:foo+bar+baz');
});

it('encodes null values as `!null`', () => {
const param = buildArgsParam({}, { key: null });
expect(param).toEqual('key:!null');
});

it('encodes nested null values as `!null`', () => {
const param = buildArgsParam({}, { foo: { bar: [{ key: null }], baz: null } });
expect(param).toEqual('foo.bar[0].key:!null;foo.baz:!null');
});

describe('with initial state', () => {
it('omits unchanged values', () => {
const param = buildArgsParam({ one: 1 }, { one: 1, two: 2 });
Expand All @@ -138,6 +189,16 @@ describe('buildArgsParam', () => {
expect(param).toEqual('obj.two:2');
});

it('sets !undefined for removed array values', () => {
const param = buildArgsParam({ arr: [1] }, { arr: [] });
expect(param).toEqual('arr[0]:!undefined');
});

it('sets !undefined for removed object properties', () => {
const param = buildArgsParam({ obj: { one: 1 } }, { obj: {} });
expect(param).toEqual('obj.one:!undefined');
});

// TODO reintroduce sparse arrays when a new version of `qs` is released
// @see https://github.com/ljharb/qs/issues/396
// it('omits unchanged array values (yielding sparse arrays)', () => {
Expand Down
50 changes: 36 additions & 14 deletions lib/router/src/utils.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { once } from '@storybook/client-logger';
import deepEqual from 'fast-deep-equal';
import qs from 'qs';
import isPlainObject from 'lodash/isPlainObject';
import memoize from 'memoizerific';
import { once } from '@storybook/client-logger';
import qs from 'qs';

export interface StoryData {
viewMode?: string;
Expand Down Expand Up @@ -37,27 +38,47 @@ interface Args {
[key: string]: any;
}

const deepDiff = (value: any, update: any): any => {
if (deepEqual(value, update)) return undefined;
export const DEEPLY_EQUAL = Symbol('Deeply equal');
export const deepDiff = (value: any, update: any): any => {
if (typeof value !== typeof update) return update;
if (Array.isArray(value)) return update;
if (update && typeof update === 'object') {
return Object.keys(update).reduce((acc, key) => {
const diff = deepDiff(value[key], update[key]);
return diff === undefined ? acc : Object.assign(acc, { [key]: diff });
if (deepEqual(value, update)) return DEEPLY_EQUAL;
if (Array.isArray(value) && Array.isArray(update)) {
if (update.length >= value.length) return update;
return [...update, ...new Array(value.length - update.length)];
}
if (isPlainObject(value) && isPlainObject(update)) {
return Object.keys({ ...value, ...update }).reduce((acc, key) => {
const diff = deepDiff(value?.[key], update?.[key]);
return diff === DEEPLY_EQUAL ? acc : Object.assign(acc, { [key]: diff });
}, {});
}
return update;
};

const encodeNullish = (value: unknown): any => {
if (value === undefined) return '!undefined';
if (value === null) return '!null';
if (Array.isArray(value)) return value.map(encodeNullish);
if (isPlainObject(value)) {
return Object.entries(value).reduce(
(acc, [key, val]) => Object.assign(acc, { [key]: encodeNullish(val) }),
{}
);
}
return value;
};

// Keep this in sync with validateArgs in @storybook/core
const VALIDATION_REGEXP = /^[a-zA-Z0-9 _-]*$/;
const validateArgs = (key = '', value: any = ''): boolean => {
if (key === null || value === null) return false;
const validateArgs = (key = '', value: unknown): boolean => {
if (value === null || value === undefined) return true; // encoded as `!null` or `!undefined`
if (key === null) return false;
if (key === '' || !VALIDATION_REGEXP.test(key)) return false;
if (typeof value === 'number' || typeof value === 'boolean') return true;
if (typeof value === 'string') return VALIDATION_REGEXP.test(value);
if (Array.isArray(value)) return value.every((v) => validateArgs(key, v));
return Object.entries(value).every(([k, v]) => validateArgs(k, v));
if (isPlainObject(value)) return Object.entries(value).every(([k, v]) => validateArgs(k, v));
return false;
};

const QS_OPTIONS = {
Expand All @@ -68,7 +89,7 @@ const QS_OPTIONS = {
};
export const buildArgsParam = (initialArgs: Args, args: Args): string => {
const update = deepDiff(initialArgs, args);
if (!update) return '';
if (!update || update === DEEPLY_EQUAL) return '';

const object = Object.entries(update).reduce((acc, [key, value]) => {
if (validateArgs(key, value)) return Object.assign(acc, { [key]: value });
Expand All @@ -79,7 +100,8 @@ export const buildArgsParam = (initialArgs: Args, args: Args): string => {
}, {} as Args);

return qs
.stringify(object, QS_OPTIONS)
.stringify(encodeNullish(object), QS_OPTIONS)
.replace(/ /g, '+')
.split(';')
.map((part: string) => part.replace('=', ':'))
.join(';');
Expand Down