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

[6.3.0rc-8] Controls not inheriting args from url. #15278

Closed
TheComputerM opened this issue Jun 17, 2021 · 25 comments
Closed

[6.3.0rc-8] Controls not inheriting args from url. #15278

TheComputerM opened this issue Jun 17, 2021 · 25 comments

Comments

@TheComputerM
Copy link

TheComputerM commented Jun 17, 2021

Describe the bug
The args in the url are not obeyed.

To Reproduce
I toggle a control named outlined
image

I copy the url and paste it into another tab.
image

The outlined control is not toggled on.

System
System:
OS: Windows 10 10.0.19041
CPU: (4) x64 Intel(R) Core(TM) i5-6200U CPU @ 2.30GHz
Binaries:
Node: 14.17.0 - C:\Program Files\nodejs\node.EXE
Yarn: 1.22.10 - ~\AppData\Roaming\npm\yarn.CMD
npm: 6.14.13 - C:\Program Files\nodejs\npm.CMD
Browsers:
Chrome: 81.0.4044.138
Edge: Spartan (44.19041.1023.0), Chromium (91.0.864.48)
npmPackages:
@storybook/addon-a11y: ^6.3.0-rc.8 => 6.3.0-rc.8
@storybook/addon-controls: ^6.3.0-rc.8 => 6.3.0-rc.8
@storybook/addon-measure: ^1.2.4 => 1.2.4
@storybook/addon-storysource: ^6.3.0-rc.8 => 6.3.0-rc.8
@storybook/addon-svelte-csf: ^1.1.0 => 1.1.0
@storybook/addon-viewport: ^6.3.0-rc.8 => 6.3.0-rc.8
@storybook/addons: ^6.3.0-rc.8 => 6.3.0-rc.8
@storybook/svelte: ^6.3.0-rc.8 => 6.3.0-rc.8
@storybook/theming: ^6.3.0-rc.8 => 6.3.0-rc.8

┆Issue is synchronized with this Asana task by Unito

@shilman
Copy link
Member

shilman commented Jun 17, 2021

Can you please create a reproduction by running npx sb@next repro, following the instructions, and linking it in your issue description? Or if you have an existing repro you can share, that would be much appreciated!

Also, do you happen to be using the storybook-builder-vite?

@shilman shilman added the svelte label Jun 17, 2021
@TheComputerM
Copy link
Author

I am not using storybook-builder-vite. Can I send you a repro using sb init?

@shilman
Copy link
Member

shilman commented Jun 17, 2021

@TheComputerM that's basically what sb repro does, but it adds a little extra information to standardize the repo for maintainers to consume

@TheComputerM
Copy link
Author

Here is repro: https://github.com/TheComputerM/url-params-storybook

@ghengeveld
Copy link
Member

The way the options are configured is deprecated (see console), but correcting it did not help. Need to look into this.

@shilman
Copy link
Member

shilman commented Jun 24, 2021

Gadzooks!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.4.0-alpha.0 containing PR #15332 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb upgrade --prerelease

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

@shilman shilman closed this as completed Jun 24, 2021
@TheComputerM
Copy link
Author

Thank you very much @shilman.

@TheComputerM
Copy link
Author

@shilman , @ghengeveld Just tested this out in storybook 6.4.0 alpha, still getting the same error. Even reinstalled the node_modules folder.

@shilman shilman reopened this Jun 24, 2021
@shilman
Copy link
Member

shilman commented Jun 24, 2021

@ghengeveld can you check out the repro above with 6.4 and let me know what you find?

@ghengeveld
Copy link
Member

ghengeveld commented Jun 26, 2021

With that repo, after upgrading with npx sb upgrade --prerelease I see the problem seems resolved for boolean and option types, but the backgroundColor is still broken.

@shilman
Copy link
Member

shilman commented Jun 28, 2021

Jiminy cricket!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.3.1 containing PR #15332 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 Jun 28, 2021
@shilman shilman reopened this Jun 28, 2021
@cellog
Copy link

cellog commented Aug 10, 2021

this is still present in 6.3.6. I reproduced this in an .mdx file using the docs plugin.

export const Component = <div>hi</div>
export const Template = (args) => {
  console.log(args);
  return (
    <div>hi</div>
  );
};

<Meta
  title="Bug"
  component={Component}
  argTypes={{
    allowedPageSizes: {
      options: ["custom", "undefined"],
      control: { type: "radio" },
      mapping: {
        undefined: undefined,
        custom: [5, 20, 50],
      },
    },
  }}
/>

<Canvas>
  <Story name="testing">{Template.bind({})}</Story>
</Canvas>

With that story I don't see the bug. The component in question has several parameters. I set the args for callbacks to dummy values, and that didn't help. I don't see any error messages

This is a hard blocker for us upgrading to version 7. We use the knobs plugin to open up stories in new windows and run cypress tests against them for integration tests. We cannot migrate our system over to controls in preparation for storybook 7 unless this is isolated and fixed. I see this is part of the 6.4 milestones and help is wanted. Is there any more information on the status of this bug?

@shilman
Copy link
Member

shilman commented Aug 10, 2021

@tmeasday is this also an issue you looked into, or am I confusing it with another high priority URL issue?

@tmeasday
Copy link
Member

I haven't looked into this issue specifically. I suspect the issue might be in the fact that we check the value for the arg in the URL matches up with the arg type, and there may be a bug here. What does the inferred argType look like in this case?

This code is being refactored in 6.4, but I think the basic mechanism is the same, so if there is a bug in what it is doing it will likely need to be fixed.

@cellog
Copy link

cellog commented Aug 11, 2021

How would I determine what the inferred argType looks like? allowedPageSizes is an array of numbers, but it's defined using a typescript generic. When undefined, it uses a default value of [10, 20, 40, 60, 100].

@shilman
Copy link
Member

shilman commented Aug 11, 2021

@cellog in a story export:

export const MyStory = (args, context) => {
  console.log(context.argTypes);
  // normal story ...
}

@tmeasday
Copy link
Member

@cellog also what does a malfunctioning URL look like for you?

@cellog
Copy link

cellog commented Aug 12, 2021

the URL looks correct (same params as a functioning version) args=allowedPageSizes:custom or something like that (I'm on a different machine atm)

@cellog
Copy link

cellog commented Oct 8, 2021

just noticing this part of the docs:

In order to protect against XSS attacks, keys and values of args specified through the URL are limited to alphanumeric characters, spaces, underscores and dashes. Any args that don't abide these restrictions will be ignored and stripped, but can still be used through code and manipulated through the Controls addon.

I think this might be what is happening - I will check now. If there is simply no error, that's a problem. At the least there should be a console.warn() that an arg is being ignored

@cellog
Copy link

cellog commented Oct 8, 2021

UPDATE! I figured out at least the source of the bug.

If you define argTypes inside a .stories.mdx it fails. If you define them inside a .stories.tsx, it succeeds. Something about mdx transformation is what's triggering this bug.

@cellog
Copy link

cellog commented Oct 8, 2021

for us, this is great news because the solution is to migrate to using .stories.tsx/.mdx instead of .stories.mdx which is much better anyways

@shilman
Copy link
Member

shilman commented Oct 8, 2021

@cellog do you have a reproduction or even a small example of an argType declaration that works in CSF/.tsx but does not work in .mdx?

@pahan35
Copy link

pahan35 commented Oct 27, 2021

We have the same error even in .stories.tsx.

Still an issue in the latest release.

Environment Info:

System:
OS: macOS 11.6
CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
Binaries:
Node: 14.17.6 - /usr/local/opt/node@14/bin/node
Yarn: 3.0.2 - /usr/local/bin/yarn
npm: 6.14.15 - /usr/local/opt/node@14/bin/npm
Browsers:
Chrome: 95.0.4638.54
Safari: 15.0
npmPackages:
@storybook/addon-actions: ^6.4.0-beta.20 => 6.4.0-beta.20
@storybook/addon-docs: ^6.4.0-beta.20 => 6.4.0-beta.20
@storybook/addon-essentials: ^6.4.0-beta.20 => 6.4.0-beta.20
@storybook/addon-links: ^6.4.0-beta.20 => 6.4.0-beta.20
@storybook/react: ^6.4.0-beta.20 => 6.4.0-beta.20

@shilman do you need any reproduction for it?

@pahan35
Copy link

pahan35 commented Oct 27, 2021

As I see, we take initial args state here

let [args, updateArgs, resetArgs] = useArgs(storyId, context);
via useArgs() where
const storyContext = context.getStoryContext(story);
we take them them from context.getStoryContext(story) which returns only default args for a story, and subscribes to story arg update here
channel.on(Events.STORY_ARGS_UPDATED, cb);
, but it's never triggered.

It looks like, to fix the issue, we need to storyArgsUpdated event with args from URL if they provided after adding its listener in ArgsTable.

I'll submit a PR with this changes.

@shilman please let me know what do you think about it.

@shilman
Copy link
Member

shilman commented Nov 8, 2021

Boo-yah!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.4.0-beta.30 containing PR #16508 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb upgrade --prerelease

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

@shilman shilman closed this as completed Nov 8, 2021
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

6 participants