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

@storybook/server inlcude global parameters when requesting story to the server #13133

Closed
santialbo opened this issue Nov 16, 2020 · 10 comments
Closed

Comments

@santialbo
Copy link
Contributor

I want to use storybook/server to show stories of emails generated with mjml.

Each of the emails has an English and a Spanish version and also an HTML and a text version. For this reason I created a couple globalTypes with the toolbar-addon.
However the selected values in these dropdowns are not passed as parameters to the rendering server.

Is there a way of passing these global parameters to the server as query parameters?

@santialbo
Copy link
Contributor Author

It looks like this would be easy to add. I can do it myself and open a PR. My only question would be what's the preferred naming of the query parameters.
Options:

  • pass them like any other parameters.
  • prefix them with something like globals.$param

If I'm not wrong, globals can be gotten from the RenderContext

export async function renderMain({
id,
kind,
name,
showMain,
showError,
forceRender,
parameters,
storyFn,
args,
argTypes,
}: RenderContext) {

and they can be passed to fetchStoryHtml as a new fourth parameter.

@jonspalmer
Copy link
Contributor

I need to learn up on global args and toolbars.

My first instinct would - Is it necessary to use globals vs just providing them as a regular parameter? This can be done at a high level and I think works with args etc.

Second - If we use globals
a) I don't think we should namespace them at all - just send them with the other parameters. The server renderer shouldn't have to know about globals. That there are two ways to provide them is a Storybook concern.
b) I think we'd want a mechanism for opting in to this behavior maybe with allow/deny lists of the global params to include?

@shilman
Copy link
Member

shilman commented Nov 17, 2020

@jonspalmer In the story context we namespace these separately { args, globals }. We don't need to mirror that in the query parameters, but a globals.X convention as proposed above sounds reasonable to me.

@santialbo
Copy link
Contributor Author

Another possibility is just passing the whole render context to the fetchStoryHtml function so it would be up to the user to decide if to pass the globals and how by writing a custom fetchStoryHtml.

@santialbo
Copy link
Contributor Author

About global variables it's just a nice UX feature from Storybook that allows to move controls from the "Controls" section to the top toolbar.
It's meant for variables that are common in all your stories. Think things like locale, theme, etc.
In my case since I'm rendering emails in different languages I use it for the language and for the "content type" (html or text).

@shilman
Copy link
Member

shilman commented Nov 17, 2020

Yup what @santialbo said. @jonspalmer you can read more here: https://storybook.js.org/docs/react/essentials/toolbars-and-globals

Basically the mechanism is very similar to args, except that it's global to Storybook and not specific to the story/component.

@jonspalmer
Copy link
Contributor

Thanks for the great info. I can see the need to support this. My sense is we should do a few things:

  1. pass globals into fetchStoryHtml so that any custom overrides can use them
  2. Default implementation of fetchStoryHtml should merge the globals with params when we build the server query params
  3. add a new configuration option that could be set in .storybook/preview.js that controls which globals which globals get merged. Something like
export const parameters = {
  server: {
    url: `http://localhost:${port}/storybook_preview`,
    globals: '*'
  },
};

Options would be a "*", and Array of strings - acts as an allow list. We could also allow an object that acts as a allow list but also renames the params.

Thoughts?

@santialbo
Copy link
Contributor Author

I agree on 1 and 2.
On 3, I think that if the user really wants to control which variables are passed they could define a custom fetchStoryHtml to do that. What do you think?

I went ahead and implemented a simple solution where I pass the context as fourth parameter in case someone might need it. If you prefer to just pass globals I'd be happy to update it.

@stale
Copy link

stale bot commented Dec 25, 2020

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@stale stale bot added the inactive label Dec 25, 2020
@shilman
Copy link
Member

shilman commented Jan 11, 2021

Olé!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.2.0-alpha.11 containing PR #13158 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb upgrade --prerelease

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants