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

url params don't work for some args (protected names maybe?) #14508

Closed
chadlavi-casebook opened this issue Apr 7, 2021 · 13 comments
Closed

url params don't work for some args (protected names maybe?) #14508

chadlavi-casebook opened this issue Apr 7, 2021 · 13 comments

Comments

@chadlavi-casebook
Copy link

chadlavi-casebook commented Apr 7, 2021

Describe the bug

After reading that storybook supports controlling args through URL parameters [1] [2], I am trying it out, because I had a great use case for linking to a particular set of props for a component from my release notes.

However, it seems that when the prop you want to control is called name, or when you want to control the children prop, this method doesn't work.

To Reproduce

I have a prop that has a type like this:

name: 'foo' | 'bar' | 'baz' | ...

I tried a URL param like this:

&args=name:foo

but it had no effect.

There are, however, other props that do work just fine, and they have similar types. For example I have props like

color?: 'default' | 'primary' | 'secondary' | 'inherit'

and passing

&args=name:foo;color:secondary

does correctly set the color, even though the name prop gets ignored.

This leads me to suspect that it's the name name itself that is causing the issue here. adding a new name prop to other components seems to suggest this is reproducible.

I'm also experiencing this with the children prop.

@chadlavi-casebook
Copy link
Author

This is reproducible in Storybook's own DS storybook: https://5ccbc373887ca40020446347-qrbicufmsd.chromatic.com/?path=/story/button--basic&args=children:hello;appearance:outline

that should show a button with appearance set to outline (which works correctly) and its children set to "hello", but the children arg here has no effect.

@ghengeveld
Copy link
Member

ghengeveld commented Apr 7, 2021

I'm not quite sure the DS exhibits the same issue per se. Your problem appears to be with TS inference. I'm going to need more info to debug this. Could you run the following in your Storybook (iframe context) and post the resulting JSON?

let { args, argTypes } = __STORYBOOK_STORY_STORE__._stories[__STORYBOOK_STORY_STORE__._selection.storyId]
console.log(JSON.stringify({ args, argTypes }, null, 2))

I suspect that the name field is reported as object while color is a string.

I've confirmed a bug in restoring object string values from the URL. If you set "foo" in a JSON (object) field it will sync that to the URL, but if you refresh the page it will not properly restore from that URL. Looks like a problem with mapArgsToTypes.

@chadlavi-casebook
Copy link
Author

chadlavi-casebook commented Apr 8, 2021

@ghengeveld thanks for getting back to me!

here's what I get (i truncated the list of all possible name values since it's quite long):

{
  "args": {
    "name": "casebook"
  },
  "argTypes": {
    "name": {
      "control": {
        "type": "select",
        "options": [
          "casebook",
          "drag-handle",
          "500px",
          
          ...

          "youtube-square",
          "youtube",
          "zhihu"
        ]
      },
      "type": {
        "name": "other",
        "required": true,
        "value": "\"function\" | \"search\" | \"text\" | \"image\" | \"code\" | \"h1\" | \"h2\" | \"h3\" | \"h4\" | \"link\" | \"map\" | \"table\" | \"th\" | \"video\" | \"circle\" | \"filter\" | \"line\" | \"marker\" | \"mask\" | \"stop\" | ... 2291 more ... | \"drag-handle\""
      },
      "name": "name",
      "required": true,
      "description": "The icon name",
      "sbType": {
        "name": "other",
        "value": "\"function\" | \"search\" | \"text\" | \"image\" | \"code\" | \"h1\" | \"h2\" | \"h3\" | \"h4\" | \"link\" | \"map\" | \"table\" | \"th\" | \"video\" | \"circle\" | \"filter\" | \"line\" | \"marker\" | \"mask\" | \"stop\" | ... 2291 more ... | \"drag-handle\""
      },
      "table": {
        "type": {
          "summary": "\"function\" | \"search\" | \"text\" | \"image\" | \"code\" | \"h1\" | \"h2\" | \"h3\" | \"h4\" | \"link\" | \"map\" | \"table\" | \"th\" | \"video\" | \"circle\" | \"filter\" | \"line\" | \"marker\" | \"mask\" | \"stop\" | ... 2291 more ... | \"drag-handle\""
        },
        "defaultValue": null
      }
    },
    "color": {
      "control": {
        "type": "radio"
      },
      "options": [
        "default",
        "inherit",
        "primary",
        "destructive",
        "secondary"
      ],
      "name": "color",
      "type": {
        "required": false,
        "name": "enum",
        "value": [
          "default",
          "inherit",
          "primary",
          "destructive",
          "secondary"
        ]
      },
      "required": false,
      "description": "By default an icon inherits color from its parent element. Use this prop to set the icon color independently. The DS default color for icons is #748089. 'primary' is #006EB9, and 'secondary' is #20A49A.",
      "sbType": {
        "name": "enum",
        "value": [
          "default",
          "inherit",
          "primary",
          "destructive",
          "secondary"
        ]
      },
      "table": {
        "type": {
          "summary": "\"default\" | \"inherit\" | \"primary\" | \"destructive\" | \"secondary\""
        },
        "defaultValue": null
      }
    },
    "fixedWidth": {
      "control": {
        "type": "boolean"
      },
      "name": "fixedWidth",
      "type": {
        "required": false,
        "name": "boolean"
      },
      "required": false,
      "description": "Use to set multiple icons to the same fixed width.",
      "sbType": {
        "name": "boolean"
      },
      "table": {
        "type": {
          "summary": "boolean"
        },
        "defaultValue": null
      }
    },
    "forwardClassNames": {
      "control": {
        "type": "object"
      },
      "name": "forwardClassNames",
      "type": {
        "required": false,
        "name": "other",
        "value": "string[]"
      },
      "required": false,
      "description": "classNames provided by parent component to override the styling of the icon",
      "sbType": {
        "name": "other",
        "value": "string[]"
      },
      "table": {
        "type": {
          "summary": "string[]"
        },
        "defaultValue": null,
        "disable": true
      }
    },
    "margin": {
      "control": {
        "type": "radio"
      },
      "options": [
        "default",
        "medium",
        "left",
        "right",
        "dense"
      ],
      "name": "margin",
      "type": {
        "required": false,
        "name": "enum",
        "value": [
          "default",
          "medium",
          "left",
          "right",
          "dense"
        ]
      },
      "required": false,
      "description": "Sets left and right margins on the icon. 'default' is 16px, 'dense' is 8px, and 'medium' is 24px.",
      "sbType": {
        "name": "enum",
        "value": [
          "default",
          "medium",
          "left",
          "right",
          "dense"
        ]
      },
      "table": {
        "type": {
          "summary": "\"default\" | \"medium\" | \"left\" | \"right\" | \"dense\""
        },
        "defaultValue": null
      }
    },
    "size": {
      "control": {
        "type": "select"
      },
      "options": [
        "lg",
        "sm",
        "xs",
        "2x",
        "3x",
        "4x"
      ],
      "name": "size",
      "type": {
        "required": false,
        "name": "enum",
        "value": [
          "lg",
          "sm",
          "xs",
          "2x",
          "3x",
          "4x"
        ]
      },
      "required": false,
      "description": "By default an icon inherits fontSize from its parent element. Use this prop to set the icon size independently. Sets the icons size in ems, where 'xs' is .75em, 'sm' is .875em, 'lg' is 1.33em, and '2x' through '4x' are 2em through 4em.",
      "sbType": {
        "name": "enum",
        "value": [
          "lg",
          "sm",
          "xs",
          "2x",
          "3x",
          "4x"
        ]
      },
      "table": {
        "type": {
          "summary": "\"lg\" | \"sm\" | \"xs\" | \"2x\" | \"3x\" | \"4x\""
        },
        "defaultValue": null
      }
    },
    "title": {
      "control": {
        "type": "text"
      },
      "name": "title",
      "type": {
        "required": false,
        "name": "string"
      },
      "required": false,
      "description": "Icon title",
      "sbType": {
        "name": "string"
      },
      "table": {
        "type": {
          "summary": "string"
        },
        "defaultValue": null
      }
    },
    "type": {
      "control": {
        "type": "radio"
      },
      "options": [
        "brands",
        "duotone",
        "light",
        "regular",
        "solid"
      ],
      "name": "type",
      "type": {
        "required": false,
        "name": "enum",
        "value": [
          "brands",
          "duotone",
          "light",
          "regular",
          "solid"
        ]
      },
      "required": false,
      "description": "The icon type",
      "sbType": {
        "name": "enum",
        "value": [
          "brands",
          "duotone",
          "light",
          "regular",
          "solid"
        ]
      },
      "table": {
        "type": {
          "summary": "\"brands\" | \"duotone\" | \"light\" | \"regular\" | \"solid\""
        },
        "defaultValue": {
          "summary": "light"
        }
      }
    },
    "testID": {
      "control": {
        "type": "text"
      },
      "name": "testID",
      "type": {
        "required": false,
        "name": "string"
      },
      "required": false,
      "description": "Custom data attribute for testing purposes",
      "sbType": {
        "name": "string"
      },
      "table": {
        "type": {
          "summary": "string"
        },
        "defaultValue": null
      }
    }
  }
}

@shilman
Copy link
Member

shilman commented Apr 9, 2021

Jiminy cricket!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.2.6 containing PR #14511 that references this issue. Upgrade today to the @latest NPM tag to try it out!

npx sb upgrade

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

@shilman shilman closed this as completed Apr 9, 2021
@chadlavi-casebook
Copy link
Author

the latest version (6.2.7) does not address this. It still does not work.

@szymonryczek
Copy link

szymonryczek commented Apr 12, 2021

looks like I have the same problem. For arg named date can't pass value via URL. It gets undefined. Before that, I had the same problem with the select control type but it turned out that it was enough to move options from control according to updated documentation. In the case of the date arg, it seems there is only a problem with the name of that argument. Changing a parameter directly from the control addon works properly. Below is a code snippet, but the structure seems to be correct.

date: {
  control: {
    type: 'date',
  },
  defaultValue: Date.now(),
},

@aantonyuk
Copy link

I also see the same issue but with boolean attributes. When I switch to toggle from true to false it's work perfect but when I refresh the page I see the error message.

3

@antoniosZ
Copy link

antoniosZ commented May 13, 2021

I think I have a similar issue since #6133 and #13803
before that, I was using the updateArgs hook to update dynamically the args:

import { useArgs } from '@storybook/api';

const [args, updateArgs, resetArgs] = useArgs();

// To update one or more args:
updateArgs({ children: '<b>test</b>' });

through an addon I made, that was driven by my E2E testing.
With that, I could dynamically change the children arg during the E2E testing with any HTML value needed.

Now, because it syncs the arg values with the URL and therefore sanitizes them,
I get at the very least a warning in the console... (Omitted potentially unsafe URL args.)

Can an option be added to turn off this sync args to URL feature?
I really do not need it and it creates breaking changes in my case, even though I used storybook within its "allowed" use.

As for this solution
https://storybook.js.org/docs/react/writing-stories/args#mapping-to-complex-arg-values
it will force me to have to refactor all my E2E tests, as if, this is a breaking change and it will make the whole use of dynamically updating args by E2E tests at run-time, really cumbersome.

Additionally, this now causes a navigate event, which re-renders the whole page and any DOM modifications that I might have done, are gone every time I update the args...

@shilman
Copy link
Member

shilman commented May 14, 2021

@antoniosZ can you open a new issue with a repro using npx sb@next repro? it sounds to me like you're experiencing a bug, not a breaking change.

@aalpgiray
Copy link

I have just realised switching from react-docgen-typescript to react-docgen solves this. I don't know where to open this bug report. You can checkout this documentation if you don know how to set this parameter.

https://storybook.js.org/docs/react/configure/typescript

@TheBeerCake
Copy link

Switching to react-docgen not solve it for me . When I open the story in canvas it's a coin toss which URL params will work.

@dewezlopez
Copy link

dewezlopez commented Jan 12, 2022

I have just realised switching from react-docgen-typescript to react-docgen solves this. I don't know where to open this bug report. You can checkout this documentation if you don know how to set this parameter.

https://storybook.js.org/docs/react/configure/typescript

Using react-docgen worked for me. Thanks for the tip!

edit:

A caveat discovered here is if you're using ArgsTable in your docs, then they wont be picked up.

image

@shilman
Copy link
Member

shilman commented Jun 8, 2023

We’re cleaning house! Storybook has changed a lot since this issue was created and we don’t know if it’s still valid. Please open a new issue referencing this one if:

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

9 participants