Storybook: API.setQueryParams not works as expected

[leoyli]

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.

[tmeasday]

It is no longer intended that it changes the actual URL, this API is kept for back-compat but we do not want to affect the URL any more.

Arguably we should save the state to session storage however. Also I am not sure about the other issues @leoyli mentioned. @shilman you created the back-compat API right, perhaps you can take a first pass over this?

[leoyli]

@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. 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.

[shilman]

@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?

[leoyli]

@shilman, I see what you are saying here. I believe I would be able to do the same if the query is shared internally among other addons (just need to avoid the naming collisions).

[andyindahouse]

Hi @leoyli, did you get some workaround to this? it would be very helpful.

[andyindahouse]

How can I access to knobs-params in a custom-addon? I need to generate a URL to share it, with knobs and other custom parameters but I don't know how to get the knobs from custom-addon, when I use getQueryParam i need to know the key, but with knobs I don´t know the selected story, neither his keys and when I use getUrlState I don't get all queryParams.

Can someone give me a hand with this? Thanks in advance.

[shilman]

@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.

[uriel-sf]

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).

[pandaiolo]

Hi there! I have a custom addon that is broken by Storybook update to v5 because the setQueryParam is not... well, setting query params anymore. (same use case as @leoyli above with shareable URLs)

What do you suggest as a workaround? When I manually add query params, they get removed by Storybook. 😕 (Edit: did not read @leoyli suggestion, thanks! still not native URL copy/paste)

@tmeasday

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

Are there actually users complaining about this? I suspect most people just type the initial URL of wherever Storybook is hosted, don't you think? For us it is www.domain.com/storybook/. I do not expect my visitors to 1. type any other full URL outside copy&paste 2. pay attention to the current query params

Do those assumptions make sense?

The idea of adding an option to persist setQueryParams to the URL (opt-in SB4 API) sounds good to me!

[uriel-sf]

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

[milesj]

@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.

[tmeasday]

And? Who cares? That's the entire point of query params.

I assure you people care. That's why we changed it in the first place.

Anyway, I just said there is a tension, not that absolutely didn't want to support it--basically we don't want addons updating the URL "just because" and leading to the situation we had before where the storybook URL was super long and not readable and full of UI-specific things that really didn't need to be saved there. I understand the use case I think we should solve it.

@ndelangen -- still wondering about your thoughts on this, as originator of the change?

[milesj]

At minimum, I think enabling/disabling query params can be an option. Consumers who don't want the ugly params can disable it, and consumers who do want params can enable it.

[tmeasday]

Another idea I heard mentioned was to show a spot on the UI where you can get a "full" URL that includes all the bits and pieces to reproduce the current state.

I'm not quite sure if there is a benefit to having two URLs, am interested in people's thoughts here.

[shilman]

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

Steps to make this happen:

• [ ] Elevate this "copy URL" function out of the knobs UI and into the main UI
• [ ] Ensure that URL parameters are handled properly and make sure it's tested so we don't regress again (I fixed this already in the midst of the 5.0 release, but it broke again in the process of shipping?). Already tracked at https://github.com/storybooks/storybook/issues/6075
• [ ] Related: https://github.com/storybooks/storybook/issues/6133

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

[leoyli]

I think this sharing ability is very important, especially when we need to reproduce a specific state for visual smoke testing too! I am expecting the standalone mode (the icon on the top right) of a story can carry forward these query information directly (so we may just open it or copy its link directly). -> i.e. what you seen in the preview should be able transfer the state easily over to the standalone page.

---

Edit:

It actually did work in the stand alone page if I manually specify the query parameters... so I guess it is still backward compatible in that sense; but just as I suggested, the link in the manager app is not too helpful for the moment. I have to manually control in the URL to get the state I wanted. For people don't have time to dive into the source code to see how they are actually setup in a given addon, that can be very painful.

[pandaiolo]

@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.

[shilman]

@pandaiolo Yes, I'm proposing a clean URL that shows up in the toolbar and a "sharing URL" that can be generated in the UI. That's what's supposed to be implemented in the UI right now, but (1) it's apparently buggy, and (2) the "copy URL" is only available in the knobs UI, which I think is the wrong place for it.

@ndelangen Do you want to weigh in here? I think you're the one who architected this change.

[milesj]

I'm open to any approach that allows the consumer to use the full URL somehow. We write custom addons and need to persist our state via query params. At the moment it's very difficult.

[shilman]

@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 😆)

[ndelangen]

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

[shilman]

I'm good with that solution @ndelangen provided that there's an easy way to specify what state gets into the URL and what is kept internal. Those ugly share URLs would get even uglier if we persisted all the state there 😉

[leoyli]

@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.

[ndelangen]

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);

[tmeasday]

Technically, how would it work for a preview-only URL to open up with knobs parameters on the URL? Aren't those parameters read by the manager and sent to the knobs in the preview side via the channel? Would this require an overhaul of how knobs works to be feasible?

[shilman]

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.

[milesj]

@shilman Is there a migration guide for 5.1? Seeing as how there are 25 alpha releases... it sounds intense.

[shilman]

@milesj I release an alpha once every few days. The releases typically contain new features, bugfixes ahead of getting patched back into master, and dependency updates. Right now, the only major migration is for React-Native and it's documented here:

https://github.com/storybooks/storybook/blob/next/MIGRATION.md#from-version-50x-to-51x

Since this project has so many contributors, it usually requires some coordinated effort to get the migration instructions right during beta releases, and I'll make sure that happens before 5.1 goes out.

[milesj]

Awesome thank you!

[shilman]

Please give this a try and let me know how it works for you. If it works well and is not breaking anything I'll patch it back into 5.0 even though that’s probably technically a violation of semver. LMK what you think!

@leoyli @tmeasday @uriel-sf @pandaiolo @andyindahouse @ndelangen @jack-sf @milesj

[saibotlive]

@shilman just to add, I used to use process.browser in my application. This works fine on the stable release but returns undefined in your alpha release.

[shilman]

@saibotlive Would you mind trying out the latest prerelease https://github.com/storybooks/storybook/releases/tag/v5.1.0-rc.2 and let me know if this is still a problem for you?

[GeorgeTaveras1231]

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();