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

Generate Storybook URLs using v5+ structure #12

Merged
merged 3 commits into from
Mar 24, 2020
Merged

Generate Storybook URLs using v5+ structure #12

merged 3 commits into from
Mar 24, 2020

Conversation

yuqu
Copy link
Contributor

@yuqu yuqu commented Mar 24, 2020

Fixes #11

@yuqu yuqu requested review from ergunsh and skarahoda March 24, 2020 09:36
@jgarplind
Copy link

LGTM.

Intent is aligned with need.

Solution confirmed to work by running npm link in own repo.

src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated
`/?selectedKind=${encodedKind}&selectedStory=${encodeURIComponent(name)}`
);
} else {
url = join(this.targetUrl, `/?selectedKind=${encodedKind}`);

Choose a reason for hiding this comment

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

we can use query parameter like: urlJoin(this.targetUrl, { query: { selectedKind: encodedKind } });

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think this is necessary.

Firstly, it just makes me to repeat put same option inputs on every call (proper-url-join has troublesome API definition, the problem is not type definition but the implementation is not suitable for rest params input definition).

Secondly, it is more visible to maintenance. No need to delegate SB URL specifics into a 3rd party lib. I just use it to ensure the final concatenated string is a good lookin valid URL without redundant characters(e.g. excessive slashes).

Choose a reason for hiding this comment

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

👌

src/index.ts Show resolved Hide resolved
@yuqu yuqu requested a review from skarahoda March 24, 2020 13:25
@yuqu yuqu force-pushed the generate-v5-url branch from 9c2d5e2 to dba4589 Compare March 24, 2020 20:21
@yuqu yuqu force-pushed the generate-v5-url branch from dba4589 to dccfb09 Compare March 24, 2020 20:24
@yuqu yuqu merged commit 843d3e4 into master Mar 24, 2020
@yuqu yuqu deleted the generate-v5-url branch March 24, 2020 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

URI encoding and trailing slash problems on generated links
3 participants