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

URI encoding and trailing slash problems on generated links #11

Closed
yuqu opened this issue Mar 24, 2020 · 0 comments · Fixed by #12
Closed

URI encoding and trailing slash problems on generated links #11

yuqu opened this issue Mar 24, 2020 · 0 comments · Fixed by #12

Comments

@yuqu
Copy link
Contributor

yuqu commented Mar 24, 2020

I have the same issue as @jurrianlammerts , but I can also expand on my scenario a bit more since its not identical:

In my components.json I have specified the config as such (trailing slash intentional, even tried adding it twice.. more on that later):

"plugins": [
    {
      "name": "@zeplin/cli-connect-storybook-plugin",
      "config": {
        "url": "https://dev-next.trr.se/component-library/"
      }
    },

The plugin seems to do a decent job at picking up available stories. There are a couple of stories, first two look like this (and the plugin seems to always default to the first one):

storiesOf('Components/Button', module)
  .addDecorator(withInfo)
  .addParameters({ info: 'Button component - Primary' })
  .add('Primary - Default', () => (
    <Button onClick={action('clicked')} primary testSelector="primaryselector">
      Primary
    </Button>
  ))
  .add('Primary - Large', () => (
    <Button
      primary
      large
      onClick={action('clicked')}
      testSelector="primaryselector"
    >
      Primary
    </Button>
  ))

Now to the part where things act up..

  1. The link that the plugin produces looks like this:
    https://dev-next.trr.se/component-library?selectedKind=Components%2FButton&selectedStory=Primary%20-%20Default
    However, when I manually navigate there (no Zeplin interaction), I end up on this link: https://dev-next.trr.se/component-library/?path=/story/components-button--primary-default
    This is mostly fine and just two different ways leading to the same result, BUT the link produced by the plugin doesn't resolve correctly!
    After a closer look on the source code, I assume this is the (correct) work of the url-join library and that the issue is that my URL has a trailing slash where there shouldn't be any in order to follow standards

If I however append a trailing slash to the configured url (this is where the trailing slash config comes in), the browser (i.e. Storybook) translates [note the trailing slash] https://dev-next.trr.se/component-library/?selectedKind=Components%2FButton&selectedStory=Primary%20-%20Default to https://dev-next.trr.se/component-library/?path=/story/components-button--primary-default

...and at this point I am in the same situation as @jurrianlammerts - on the first load this image is presented:
image
However, a refresh solves the issue, and any subsequent visits to any component do not share the same problem. I'll move on to browse the Storybook repo for any such issues too.

UPDATE:
I found this in the sources tab, possibly related?
On the first load, Storybook is working with an undefined id
image

On the second/reload, we are looking at the correct source:
image

UPDATE2:
Possibly relevant Storybook issues (closed with fix/merge though): storybookjs/storybook#5925 (comment)
storybookjs/storybook#6075

YET ANOTHER UPDATE:
Given Update2, possibly this could be resolved by adopting the new Storybook URL format?

So for selectedKind we'd have to skip the urlEncoding and instead apply lowercase:

private createLink(kind: string, name?: string): Link {
        const urlEncodedKind = encodeURIComponent(kind);

        let preparedUrl;
        if (name) {
            const urlEncodedName = encodeURIComponent(name);
            preparedUrl = urljoin(this.targetUrl, `?selectedKind=${urlEncodedKind}&selectedStory=${urlEncodedName}`);
        } else {
            preparedUrl = urljoin(this.targetUrl, `?selectedKind=${urlEncodedKind}`);
        }

        return { type: LinkType.storybook, url: preparedUrl };
    }

to replace selectedKind=Components%2FButton with path=/story/components-button

& would be replaced by --

and then selectedStory=Primary%20-%20Default would be changed to primary-default again by replacing encoding with lowercase.

If this is the direction you would like to go, I'd be happy to submit a PR.

Originally posted by @jgarplind in #6 (comment)

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 a pull request may close this issue.

1 participant