storybook: API.setQueryParams not works as expected

Describe the bug Expected URL updated when api.setQueryParams get called.

To Reproduce Steps to reproduce the behavior:

  • In any add-on tooltip component, where get access with storybookAPI, call storybookAPI.setQueryParams({ foo: 'bar' }).

Expected behavior URL change to, for example: http://localhost:6006/?path=/story/test&foo=bar.

System:

  • OS: MacOS Mojave
  • Device: MacBook Air 2013-mid
  • Browser: Chrome 72
  • Framework: React 16.8.1
  • Version: v5.0.0-beta.2

Additional context Related #5271

if call the following immediately

console.log(api.getQueryParam('foo')) // `bar` printed

Therefore, it is working (previously in alpha.11 is not working though), but the URL is not changing. Also, by directly change the URL state like http://localhost:6006/?path=/story/test&foo=bez. 'bez' get printed correctly, but the URL changed to http://localhost:6006/?path=//story/test. It added additional slash after ?path mysteriously… And if refresh the page, no 'bez' get printed.

About this issue

  • Original URL
  • State: closed
  • Created 5 years ago
  • Comments: 34 (21 by maintainers)

Most upvoted comments

If the API is called set/get query params, you kinda expect it to work that way 😃 How about instead of changing its behavior, this API is slowly deprecated over several releases? That would probably be better for devs expecting the API to work like it did before (myself included).

@tmeasday Here gives me some confusions, does any reason we don’t want the URL keep the query parameter?! Ideally, I would like to share the URL to other people, let’s say for designer. That would be super useful for our communication.

The biggest reason for me to have query in URL is for testing. I have add-on work for this (e.g. &themes=dark&languages=english), this gives us the ability to mock a particular environment state that the component is reacted to (i.e theming or language). It would be nice we can have this kept when we “open the component into a new tab”, then we can copy past URL without manually typing those parameters.

In our team, we want to have at least 1 story per dummy components and they are smartly knobbed and auto-generated. For some special variants (i.e. component specific) we will have more granular-defined stories to document some important use case so we can cherry-pick them for smoke testing.

@tmeasday

I think there is a tension here with URLs becoming hideous and unreadable.

And? Who cares? That’s the entire point of query params. If I want to send a URL to a co-worker, with the exact state of the story I’m looking at, the only way to do that is with query params. At the moment, it’s impossible and it forces my co-worker to go through the same steps I did to get to the same outcome.

This is even harder to accomplish when using addons or custom implementations.

I’m somewhat disappointed by this change.

Mainly because there seems to be an over-estimated value of “pretty” URLs. And I think that is a subjective perspective – some people value sharability of state more than pretty URLs.

Makes me wonder if having an app-level setting makes sense – similar to the Show sidebar and Show addons settings. Something like Sharable urls.

Additionally, I think there is something not being considered in this issue (and in the solution proposed in #6486) which is the difference between queries in the preview and manager apps.

In my use case (which I understand is likely different than others’) I’d only like to set the query url of the manager app. There is a state that influences how my addon renders (not the story) that I’d like to make sharable. The current feature where users can get the full URL by opening the preview app (iframe) separately doesn’t totally work for me because as I said, what I’d like to make sharable is a link to the manager app to the tab of my addon, with the modified state.

One thought that I’d like to share to support different treatment of preview queries and manager queries is the already existing difference between query params for the selected story (Referring to iframe.html?id=story-id vs ?path=/story/story-id)

I think having control over the query params on both of the apps would be a great way to empower addon developers.

Something like

const [setManagerQuery, managerQuery] = useManagerQuery();
const [setPreviewQuery, previewQuery] = usePreviewQuery();

Hey devs, any ETA on when we can expect backwards-compat behavior?

@andyindahouse You should be able to listen on the channel every time the knobs update:

https://github.com/storybooks/storybook/blob/next/addons/knobs/src/KnobManager.js#L89

Then you should be able to serialize the knob values yourself:

https://github.com/storybooks/storybook/blob/next/addons/knobs/src/components/Panel.js#L107

This could all change in future releases though, so do this at your own risk. This isn’t a use-case of knobs, and is actually different from what @leoyli is asking in the original issue.

@leoyli We’re addressing this in the knobs addon as follows:

  1. When the user modifies knobs in the UI, the URL doesn’t update
  2. When the user hits the “copy URL” button in the knobs addon, it copies a big URL with query params in it
  3. When the user pastes that big URL into the browser, the knobs addon copies those params out and updates its internal state

Would you be able to do something similar to support your use case?

Son of a gun!! I just released https://github.com/storybooks/storybook/releases/tag/v5.1.0-alpha.25 containing PR #6486 that references this issue. Upgrade today to try it out!

Because it’s a pre-release you can find it on the @next NPM tag.

Closing this issue. Please re-open if you think there’s still more to do.

Ok, I’ve been doing some digging and coding… so there are 2 things I’ve been working on today: It’s about to get technical…

a few facts:

  1. Addons used to be able to write to the URL using the setQueryParams, knobs used to do this

    it made the URL long, unreadable, BUT share-able

    The need for creating a shareable url from knobs was addressed by a button implemented in the addon itself

    More addons would like to be able to use this feature

  2. V5 introduced an easy way to open the preview/iframe in a new window

    it made it easier to debug components and test on devices

    The url is generated for the new window has 1 queryParameter: id, no custom params.

  3. Addons all manage their own local state, and have really no standardized way to share state

    in V5 the storybook state management has been overhauled and addons can use a Consumer to retrieve the api and subscribe to storybook-state But there is currently no way for addons to store their state in the global storybook state.

I think there’s 2 things to do:

1 - make customQueryParameters useful again.

And it seems people want to use them for accessing specific story states. This means the customQueryParameters must be accessible in the iframe.

2 - make management of addon state easy and convenient, so they too can make use of persistence, and other useful things.


First I’m going to address the customQueryParameters.

  1. The URL of the preview will include the custom parameters
  2. The URL of the opened window will include the custom parameters
  3. There will be an client-api for reading parameters

    but setting state will need to be handed by custom code


Second I have some new API in mind for setting addon state:

api.setAddonState(addonId, state, options);

@ndelangen,

I’m also good with the above too. But I hope the “opening on a standalone tab with the current state” can also put on the table (query params are working there but just not approachable…). Reason as I’ve mentioned.

About the store part, my initial concerns is a bit on mess up the storybook internal state so it became hard to debug but maybe I just worries for no reason. After some thoughts, I have also think this could be a great idea since so far I’m relying on singleton pattern for managing addon-contexts state. Have an official way to persist the state may unlock some potential for addon creators.

Sorry for the delay on my part.

  • standardising the knobs’ copy feature would be great
  • having a piece of UI to generate a share-url
  • keeping the main url easy to read is important

What if we allowed addons to put their state into the main storybook state? This would help addon creators, since they can more easily combine their own state, with the state of other addons & storybook’s internal state using the Consumer-component.

But also, because there’s now a centralized state for all these things, we can easily serialize that state, and temporarily drop it into the url for sharing and load this state when the url contains this data (and then clean the url).

This would help addon creators and users and maintainers at the same time.

There’d be no api for setting some url, that isn’t actually shown, only an api for setting state.

Feedback welcome @shilman @tmeasday

@milesj Thanks for your patience on this & flexibility on the approach. URL handling got messed up in various ways in the v5 release and didn’t manage to get fix in the post-release bug bash. I’ll make sure it gets handled soon – on vacation this week (supposedly) but will put more urgency here (aside from just labeling as high priority, which apparently didn’t magically solve the problem 😆)

@shilman is that “copy URL” feature different from the actual browser URL?

I concur with @tmeasday , if a use case requires URL copy/paste, it seems an anti pattern to have both a real browser URL that does not contain every params, and another “secondary URL” component, either shown in the UI or hidden with a button that says “copy URL”? (even though it is not copying browser URL, but something else instead)

I also agree with @milesj on an opt-in solution to let the user decide if they would like to use query params in the URL or not.

I have a feeling that this change basically favour aesthetics over actual feature. Prioritization of Form Over Function, if you will.

I think the URLs should be clean and we should allow people to get the “full” URL for sharing.

Steps to make this happen:

@milesj Would this be an acceptable solution from your point of view?

I think there is a tension here with URLs becoming hideous and unreadable. Potentially there is still an argument for specific addons to write to the URL, maybe we should support that via setQueryParam or even .setState(.., { persistence: 'url' }).

@ndelangen I think was the driver of removing this from the URL. I am interested to hear his thoughts.