Skip to content

Commit

Permalink
Merge pull request #28315 from 43081j/pq
Browse files Browse the repository at this point in the history
Core: Migrate from `qs` to `picoquery`
  • Loading branch information
ndelangen authored Oct 1, 2024
2 parents 8d0e714 + 26659e4 commit 148fa0f
Show file tree
Hide file tree
Showing 16 changed files with 130 additions and 112 deletions.
6 changes: 3 additions & 3 deletions code/.storybook/preview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -320,9 +320,9 @@ export const parameters = {
{ color: '#ff4785', title: 'Coral' },
{ color: '#1EA7FD', title: 'Ocean' },
{ color: 'rgb(252, 82, 31)', title: 'Orange' },
{ color: 'RGBA(255, 174, 0, 0.5)', title: 'Gold' },
{ color: 'rgba(255, 174, 0, 0.5)', title: 'Gold' },
{ color: 'hsl(101, 52%, 49%)', title: 'Green' },
{ color: 'HSLA(179,65%,53%,0.5)', title: 'Seafoam' },
{ color: 'hsla(179,65%,53%,0.5)', title: 'Seafoam' },
{ color: '#6F2CAC', title: 'Purple' },
{ color: '#2A0481', title: 'Ultraviolet' },
{ color: 'black' },
Expand All @@ -338,7 +338,7 @@ export const parameters = {
'#fe4a49',
'#FED766',
'rgba(0, 159, 183, 1)',
'HSLA(240,11%,91%,0.5)',
'hsla(240,11%,91%,0.5)',
'slategray',
],
},
Expand Down
3 changes: 1 addition & 2 deletions code/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,6 @@
"@types/prettier": "^3.0.0",
"@types/pretty-hrtime": "^1.0.0",
"@types/prompts": "^2.0.9",
"@types/qs": "^6",
"@types/react-syntax-highlighter": "11.0.5",
"@types/react-transition-group": "^4",
"@types/semver": "^7.5.8",
Expand Down Expand Up @@ -386,11 +385,11 @@
"npmlog": "^7.0.0",
"open": "^8.4.0",
"picomatch": "^2.3.0",
"picoquery": "^1.4.0",
"polished": "^4.2.2",
"prettier": "^3.2.5",
"pretty-hrtime": "^1.0.3",
"prompts": "^2.4.0",
"qs": "^6.10.0",
"react": "^18.2.0",
"react-dom": "^18.2.0",
"react-draggable": "^4.4.5",
Expand Down
12 changes: 6 additions & 6 deletions code/core/src/manager-api/tests/url.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe('initial state', () => {
describe('config query parameters', () => {
it('handles full parameter', () => {
const navigate = vi.fn();
const location = { search: new URLSearchParams({ full: '1' }).toString() };
const location = { search: '?' + new URLSearchParams({ full: '1' }).toString() };

const {
state: { layout },
Expand All @@ -34,7 +34,7 @@ describe('initial state', () => {

it('handles nav parameter', () => {
const navigate = vi.fn();
const location = { search: new URLSearchParams({ nav: '0' }).toString() };
const location = { search: '?' + new URLSearchParams({ nav: '0' }).toString() };

const {
state: { layout },
Expand All @@ -45,7 +45,7 @@ describe('initial state', () => {

it('handles shortcuts parameter', () => {
const navigate = vi.fn();
const location = { search: new URLSearchParams({ shortcuts: '0' }).toString() };
const location = { search: '?' + new URLSearchParams({ shortcuts: '0' }).toString() };

const {
state: { ui },
Expand All @@ -56,7 +56,7 @@ describe('initial state', () => {

it('handles panel parameter, bottom', () => {
const navigate = vi.fn();
const location = { search: new URLSearchParams({ panel: 'bottom' }).toString() };
const location = { search: '?' + new URLSearchParams({ panel: 'bottom' }).toString() };

const {
state: { layout },
Expand All @@ -67,7 +67,7 @@ describe('initial state', () => {

it('handles panel parameter, right', () => {
const navigate = vi.fn();
const location = { search: new URLSearchParams({ panel: 'right' }).toString() };
const location = { search: '?' + new URLSearchParams({ panel: 'right' }).toString() };

const {
state: { layout },
Expand All @@ -78,7 +78,7 @@ describe('initial state', () => {

it('handles panel parameter, 0', () => {
const navigate = vi.fn();
const location = { search: new URLSearchParams({ panel: '0' }).toString() };
const location = { search: '?' + new URLSearchParams({ panel: '0' }).toString() };

const {
state: { layout },
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import qs from 'qs';
import { stringify } from 'picoquery';

export const stringifyQueryParams = (queryParams: Record<string, string>) =>
qs.stringify(queryParams, { addQueryPrefix: true, encode: false }).replace(/^\?/, '&');
export const stringifyQueryParams = (queryParams: Record<string, string>) => {
const result = stringify(queryParams);
if (result === '') {
return '';
}

return `&${result}`;
};
3 changes: 0 additions & 3 deletions code/core/src/manager/globals/exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -618,7 +618,6 @@ export default {
'getMatch',
'parsePath',
'queryFromLocation',
'queryFromString',
'stringifyQuery',
'useNavigate',
],
Expand All @@ -635,7 +634,6 @@ export default {
'getMatch',
'parsePath',
'queryFromLocation',
'queryFromString',
'stringifyQuery',
'useNavigate',
],
Expand All @@ -652,7 +650,6 @@ export default {
'getMatch',
'parsePath',
'queryFromLocation',
'queryFromString',
'stringifyQuery',
'useNavigate',
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,5 @@ export interface SelectionStore {

setSelection(selection: Selection): void;

setQueryParams(queryParams: qs.ParsedQs): void;
setQueryParams(queryParams: Record<PropertyKey, unknown>): void;
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ describe('UrlStore', () => {
);
});
it('should replace legacy parameters but preserve others', () => {
document.location.search = 'foo=bar&selectedStory=selStory&selectedKind=selKind';
document.location.search = '?foo=bar&selectedStory=selStory&selectedKind=selKind';
setPath({ storyId: 'story--id', viewMode: 'story' });
expect(history.replaceState).toHaveBeenCalledWith(
{},
Expand All @@ -49,7 +49,7 @@ describe('UrlStore', () => {
);
});
it('should ignore + keep hashes', () => {
document.location.search = 'foo=bar&selectedStory=selStory&selectedKind=selKind';
document.location.search = '?foo=bar&selectedStory=selStory&selectedKind=selKind';
document.location.hash = '#foobar';
setPath({ storyId: 'story--id', viewMode: 'story' });
expect(history.replaceState).toHaveBeenCalledWith(
Expand Down
37 changes: 18 additions & 19 deletions code/core/src/preview-api/modules/preview-web/UrlStore.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { ViewMode } from '@storybook/core/types';
import { global } from '@storybook/global';

import qs from 'qs';
import { parse, stringify } from 'picoquery';

import type { Selection, SelectionSpecifier, SelectionStore } from './SelectionStore';
import { parseArgsParam } from './parseArgsParam';
Expand All @@ -21,20 +21,16 @@ const getQueryString = ({
extraParams,
}: {
selection?: Selection;
extraParams?: qs.ParsedQs;
extraParams?: Record<PropertyKey, unknown>;
}) => {
const search = typeof document !== 'undefined' ? document.location.search : '';
const { path, selectedKind, selectedStory, ...rest } = qs.parse(search, {
ignoreQueryPrefix: true,
const search = document?.location.search.slice(1);
const { path, selectedKind, selectedStory, ...rest } = parse(search);
const queryStr = stringify({
...rest,
...extraParams,
...(selection && { id: selection.storyId, viewMode: selection.viewMode }),
});
return qs.stringify(
{
...rest,
...extraParams,
...(selection && { id: selection.storyId, viewMode: selection.viewMode }),
},
{ encode: false, addQueryPrefix: true }
);
return `?${queryStr}`;
};

export const setPath = (selection?: Selection) => {
Expand All @@ -48,10 +44,10 @@ export const setPath = (selection?: Selection) => {
};

type ValueOf<T> = T[keyof T];
const isObject = (val: Record<string, any>) =>
const isObject = (val: Record<string, any>): val is object =>
val != null && typeof val === 'object' && Array.isArray(val) === false;

const getFirstString = (v: ValueOf<qs.ParsedQs>): string | void => {
const getFirstString = (v: ValueOf<Record<PropertyKey, unknown>>): string | void => {
if (v === undefined) {
return undefined;
}
Expand All @@ -61,15 +57,18 @@ const getFirstString = (v: ValueOf<qs.ParsedQs>): string | void => {
if (Array.isArray(v)) {
return getFirstString(v[0]);
}
if (isObject(v)) {
return getFirstString(Object.values(v).filter(Boolean) as string[]);
if (isObject(v as Record<PropertyKey, unknown>)) {
return getFirstString(
Object.values(v as Record<PropertyKey, unknown>).filter(Boolean) as string[]
);
}
return undefined;
};

export const getSelectionSpecifierFromPath: () => SelectionSpecifier | null = () => {
if (typeof document !== 'undefined') {
const query = qs.parse(document.location.search, { ignoreQueryPrefix: true });
const queryStr = document.location.search.slice(1);
const query = parse(queryStr);
const args = typeof query.args === 'string' ? parseArgsParam(query.args) : undefined;
const globals = typeof query.globals === 'string' ? parseArgsParam(query.globals) : undefined;

Expand Down Expand Up @@ -103,7 +102,7 @@ export class UrlStore implements SelectionStore {
setPath(this.selection);
}

setQueryParams(queryParams: qs.ParsedQs) {
setQueryParams(queryParams: Record<PropertyKey, unknown>) {
const query = getQueryString({ extraParams: queryParams });
const { hash = '' } = document.location;
history.replaceState({}, '', `${document.location.pathname}${query}${hash}`);
Expand Down
6 changes: 2 additions & 4 deletions code/core/src/preview-api/modules/preview-web/WebView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { global } from '@storybook/global';
import { logger } from '@storybook/core/client-logger';

import AnsiToHtml from 'ansi-to-html';
import qs from 'qs';
import { parse } from 'picoquery';
import { dedent } from 'ts-dedent';

import type { View } from './View';
Expand Down Expand Up @@ -50,9 +50,7 @@ export class WebView implements View<HTMLElement> {
// Special code for testing situations
if (typeof document !== 'undefined') {
// eslint-disable-next-line @typescript-eslint/naming-convention
const { __SPECIAL_TEST_PARAMETER__ } = qs.parse(document.location.search, {
ignoreQueryPrefix: true,
});
const { __SPECIAL_TEST_PARAMETER__ } = parse(document.location.search.slice(1));
switch (__SPECIAL_TEST_PARAMETER__) {
case 'preparing-story': {
this.showPreparingStory();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,22 +127,18 @@ describe('parseArgsParam', () => {
});

it('parses single object in array', () => {
const args = parseArgsParam('arr[].one:A;arr[].two:B');
const args = parseArgsParam('arr[0].one:A;arr[0].two:B');
expect(args).toStrictEqual({ arr: [{ one: 'A', two: 'B' }] });
});

it('parses multiple objects in array', () => {
expect(parseArgsParam('arr[0].key:A;arr[1].key:B')).toStrictEqual({
arr: [{ key: 'A' }, { key: 'B' }],
});
expect(parseArgsParam('arr[0][key]:A;arr[1][key]:B')).toStrictEqual({
arr: [{ key: 'A' }, { key: 'B' }],
});
});

it('parses nested object in array', () => {
expect(parseArgsParam('arr[].foo.bar:val')).toStrictEqual({ arr: [{ foo: { bar: 'val' } }] });
expect(parseArgsParam('arr[][foo][bar]:val')).toStrictEqual({ arr: [{ foo: { bar: 'val' } }] });
expect(parseArgsParam('arr[0].foo.bar:val')).toStrictEqual({ arr: [{ foo: { bar: 'val' } }] });
});

describe('key sanitization', () => {
Expand All @@ -164,8 +160,6 @@ describe('parseArgsParam', () => {
expect(parseArgsParam('a/b:val')).toStrictEqual({});
expect(parseArgsParam('a\\b:val')).toStrictEqual({});
expect(parseArgsParam('a|b:val')).toStrictEqual({});
expect(parseArgsParam('a[b:val')).toStrictEqual({});
expect(parseArgsParam('a]b:val')).toStrictEqual({});
expect(parseArgsParam('a{b:val')).toStrictEqual({});
expect(parseArgsParam('a}b:val')).toStrictEqual({});
expect(parseArgsParam('a?b:val')).toStrictEqual({});
Expand All @@ -185,14 +179,10 @@ describe('parseArgsParam', () => {

it('also applies to nested object keys', () => {
expect(parseArgsParam('obj.a!b:val')).toStrictEqual({});
expect(parseArgsParam('obj[a!b]:val')).toStrictEqual({});
expect(parseArgsParam('arr[][a!b]:val')).toStrictEqual({});
expect(parseArgsParam('arr[0][a!b]:val')).toStrictEqual({});
});

it('completely omits an arg when a (deeply) nested key is invalid', () => {
expect(parseArgsParam('obj.foo.a!b:val;obj.foo.bar:val;obj.baz:val')).toStrictEqual({});
expect(parseArgsParam('obj.foo[][a!b]:val;obj.foo.bar:val;obj.baz:val')).toStrictEqual({});
expect(parseArgsParam('obj.foo.a!b:val;key:val')).toStrictEqual({ key: 'val' });
});
});
Expand Down Expand Up @@ -247,18 +237,13 @@ describe('parseArgsParam', () => {

it('also applies to nested object and array values', () => {
expect(parseArgsParam('obj.key:a!b')).toStrictEqual({});
expect(parseArgsParam('obj[key]:a!b')).toStrictEqual({});
expect(parseArgsParam('arr[][key]:a!b')).toStrictEqual({});
expect(parseArgsParam('arr[0][key]:a!b')).toStrictEqual({});
expect(parseArgsParam('arr[]:a!b')).toStrictEqual({});
expect(parseArgsParam('arr[0]:a!b')).toStrictEqual({});
});

it('completely omits an arg when a (deeply) nested value is invalid', () => {
expect(parseArgsParam('obj.key:a!b;obj.foo:val;obj.bar.baz:val')).toStrictEqual({});
expect(parseArgsParam('obj.arr[]:a!b;obj.foo:val;obj.bar.baz:val')).toStrictEqual({});
expect(parseArgsParam('obj.arr[0]:val;obj.arr[1]:a!b;obj.foo:val')).toStrictEqual({});
expect(parseArgsParam('obj.arr[][one]:a!b;obj.arr[][two]:val')).toStrictEqual({});
expect(parseArgsParam('arr[]:val;arr[]:a!b;key:val')).toStrictEqual({ key: 'val' });
expect(parseArgsParam('arr[0]:val;arr[1]:a!1;key:val')).toStrictEqual({ key: 'val' });
expect(parseArgsParam('arr[0]:val;arr[2]:a!1;key:val')).toStrictEqual({ key: 'val' });
Expand Down
Loading

0 comments on commit 148fa0f

Please sign in to comment.