Add a README about client-api, specifically about args

[tmeasday]

"Args" (aka state/controls) are a new concept that replaces story state. They'll be used for future versions of knobs, context, actions and more.

Args are intended to be used in the implementation of the story (see README) if that story is parameterized.

This is a README that outlines a first phase of development of args, intended to start down the above road and support the new addon-controls (a successor to knobs).

The plan at the highest level:

1. The user can define argTypes parameters if they want
2. addon-docs uses an parameterEnhancer to add extra argTypes parameters from auto detected arg type. [This replaces the "smart controls" concept]
3. The store is initialized with argValues based on the argTypes.$.defaultValue in parameters.
4. The story is passed the values via context.args
5. Controls can use the argValues APIs to change them and re-render the story.

Interested in people's thoughts in general.

@atanasster in particular I wanted to discuss (based on your earlier comments):

A. The "reset" event -- I don't currently have this or an event to reset an individual arg. I think we can add this in a near-term iteration.

B. "on click" event -- I'm not sure I entirely understand the use case you have for this (perhaps you can tell me more?), but there's a larger issue around other functional args (in particular for actions) that we need to mull on a bit more. For now, we expect users will not use args for functional props.

C. You requested to keep the control (/arg) values alongside the type information. I can see the benefit but it is much simpler this way; the use of a parameter to control arg types maps really well to the prop table and other related things. It doesn't seem that hard to look both in context.parameters.argTypes and context.args to get the full picture. Is there something I am missing here?

D. Continuing on that -- do you think that setting the initial value for arg X based on parameters.argTypes[X].defaultValue is going to be sufficient? It seems very simple as a starting point, but I wonder if we've missed something.

[github-actions]

	Fails
🚫	PR is not labeled with one of: ["cleanup","doc-dependencies:update","BREAKING CHANGE","feature request","bug","documentation","maintenance","dependencies:update","dependencies","other"]

Generated by 🚫 dangerJS against 448a65d

[atanasster]

@tmeasday - honestly I am completely lost at this point and have no idea what you are asking me for. No malicious thoughts or anything, please don't go into defensive mode - just no idea.

I made a full implementation of exactly what I wanted to accomplish and it is waiting for a PR, we did discuss it privately and I thought I made my opinions pretty cleat - so my exact "feedback" is out there.
My honest opinion (and I did work my ### off to make it happen so I might be biased a bit) - what I did is what an "addon-controls" for sb 6.0 should be- and nothing less.

Can you please tell me did you see my work running, are there any of the features that you disagree with? I mean if something in my implementation is wrong I am ok to discuss it - but otherwise I really dont know what we are discussing.

In the meantime, I have my exact same implementation working with 5.3 at 99% (yep, hacked my way through it), so for me personally if you are asking me if a generic story state is of benefit - for me its not anymore, not sure if I have seen anyone else asking for it either. What I have seen is people asking for "knobs-2",

[tmeasday]

I made a full implementation of exactly what I wanted to accomplish and it is waiting for a PR, we did discuss it privately and I thought I made my opinions pretty cleat - so my exact "feedback" is out there.
My honest opinion (and I did work my ### off to make it happen so I might be biased a bit) - what I did is what an "addon-controls" for sb 6.0 should be- and nothing less.

Well sure, but in your branch you make a bunch of changes to core that are specific to what you are doing, where as here I am just trying to generalise it in a way that makes sense given the bigger picture of the original design (as sketched in the "Declarative Knobs and Controls" gdoc, as well as other places over the last while), and to support other features such as actions and contexts.

Basically the idea is that "args" in the store pretty neatly replaces your "controls" in the store, with some differences, pretty much as discussed in the original comment in this PR. That's why I'm seeking your feedback to check if those differences are deal breakers or if I've missed anything else.

Can you please tell me did you see my work running, are there any of the features that you disagree with? I mean if something in my implementation is wrong I am ok to discuss it - but otherwise I really dont know what we are discussing.

Of course I am 100% talking about implementation details in core. I don't really mind what your addon does, in a way that's totally up to you and I think it looks really cool.

[atanasster]

@tmeasday - maybe our wires are crossed. I had never seen "Declarative Knobs and Controls" - i was just "assigned" to make useStoryState work without any discussion if I might have an opinion about. You know the later story...

To put the scene it place: for me - controls/properties whatever you call you it are 1st class, but if you have other use cases about "Args"- please share. But my opinion - and I have been been many years in the testing industr, I really feel this is the right choice.

Anyway - I really like what I did with the controls so take it as personal opinion

Another perspective: In "react" constructor the first parameter are props, the second is context.
So it seems to me very natural to follow that.

And specifically for testing scenarios with the storybook concept of "stories" again to me it seems its the only legitimate use case.

In any case - I am 100% convinced that I am correct (I might still be wrong, just my conviction and I am way ahead un terms of working on the subject) - so if you are asking for any divergence from my blueprint, I am firmly convinced that you are making a mistake.

[atanasster]

I don't really mind what your addon does,

I really dont appreciate that feedback, just so you know. It took a lot of my time, so either say what you don;t like or give the old man some props :)

[tmeasday]

I really dont appreciate that feedback, just so you know. It took a lot of my time, so either say what you don;t like or give the old man some props :)

Oh! Sorry! I didn't mean it that way. I meant you can make it do what every you want and "I won't mind". Not that I don't like it.

[atanasster]

Well sure, but in your branch you make a bunch of changes to core that are specific to what you are doing,

That are specific to what I was asked to do. When we spoke privately I told you that my approach can be generalized and I would gladly do it if there is any support from you guys
Do you realize that I have only been been criticized and never thanked in that last couple of weeks - for trying to help you guys,
Again I really resent you putting blame on me...

[tmeasday]

I'm not sure what you mean by "blame"? This it a totally normal process for making an important change to a core API of Storybook. Things move slowly in large projects that have lots of users and an ecosystem of addons and 3rd party tools.

When we spoke privately I told you that my approach can be generalized and I would gladly do it if there is any support from you guys

I think we got our wires crossed here because what I took away from that meeting is that we needed to do the generalisation as you were frustrated with the process and ready to walk away.

Which I am happy to do because this all fits into the larger plan that was already in place (as I mentioned in the google doc which I am pretty sure you saw!). That is what I am doing here.

In any case - I am 100% convinced that I am correct (I might still be wrong, just my conviction and I am way ahead un terms of working on the subject) - so if you are asking for any divergence from my blueprint, I am firmly convinced that you are making a mistake.

I'm not sure what I can do with that man, if that's your feedback we may as well stop talking 🤷‍♂

[atanasster]

yeah, maybe we speak differently.
So specific:

• I really never review code (if that's what you asked me to do?), I might be old school, but I feel its everyone's preference how they code. Nothing against you I just can't do that.
• The approach is sound overall (well - I introduced it)
• Noting personal but I dont need a generalized story state

[atanasster]

I'm not sure what I can do with that man, if that's your feedback we may as well stop talking

Hmm - actually thats the part I like the best - you convince me of your opinion, I argue etc and we reach a best.
Is it really that bad to have a passion and opinion for what you do?

[atanasster]

Which I am happy to do because this all fits into the larger plan that was already in place (as I mentioned in the google doc which I am pretty sure you saw!).

wires definitely crossed - I did not see it until much later, Initially I just wanted to help out and was assigned to make 'useStoryState' work . I saw it only after I had my own draft document that I sent you guys, but if you didn't know I was unaware I can understand also your frstration.

[atanasster]

On the good side, check it out with 5.3 (probably works with earlier versions too):

https://ccontrols.github.io/component-controls

My only remaining issues are with the source-loader/mdx-compiler, the story state is a bit hacked :)

[tmeasday]

I really never review code (if that's what you asked me to do?),

Actually I was just hoping you'd look at this README and the comment at the top of the thread ;)

Noting personal but I dont need a generalized story state

Yeah, so this isn't a generalized story state, this is (one reason) why we've renamed it to args. The idea is very much the same as controls and what was originally outlined in the gdoc.

Hmm - actually thats the part I like the best - you convince me of your opinion, I argue etc and we reach a best.
Is it really that bad to have a passion and opinion for what you do?

Not at all! But I mean I asked you to review something not dissimilar to what you implemented and what we had discussed a few days ago, and outlined the specific differences and why I thought they were OK, and you responded saying anything not 100% the same as what you did was a bad idea. That's not really convincing anyone of anything if you ask me.

I was unaware I can understand also your frstration

I'm not frustrated, just trying to get your feedback!

[atanasster]

and you responded saying anything not 100% the same as what you did was a bad idea.

Tom, thats a way of speaking - I love "ideas" debates and I am trying to entice you :) You know this product and industry. Tell me your take on what is better - thats how progress works, no? Besides, my implementation is the only thing out there we can discuss - I assume we all have the right idea that controls need to be declarative and Idont want to take credit for it, but I am introducing certainly some innovations with faker random data.

Oth, hate coding debates like "your variable names suck", or "your code is not worthy of an alpha version - those are not not nice things right?

[atanasster]

Not at all! But I mean I asked you to review something not dissimilar to what you implemented and what we had discussed a few days ago, and outlined the specific differences and why I thought they were OK, and you responded saying anything not 100% the same as what you did was a bad

Again sorry if I misunderstood - but from a few days ago I have no idea what exactly from my PR will be accepted or merged. When I asked a few days ago what will make it into 6.0, I did not get an an answer that I could understand. Possibly my misunderstanding, and sorry if it way but I went ahead and am implementntting a version that does not depend on any changes in 6.0.
So when I say I dont need any changes - its just for me, very possibly other users need this.

[atanasster]

Here is the official review i guess i thought it was clear from my work on the controls addon:

• context.args does not work for me and will create incompatible stories. Here is what I would like to have:

export const story = props => <Button {...props} />;

Mixing up generic story state/arguments and control properties does not seem right for me and is currently a "deal-breaker" why I can not use this.
-- adding the 'smart-controls' to prop types - no, I did no ask for such thang. Maybe a misunderstanding - I did add a type field tp propDefs since it was//is being used mostly for display and the actual type information was lost.
-- resetValues - I dont need this anymore. It was initially in there as I thought it would help other implementations of knobs.
-- events like onClick - mostly for compatibility with knobs-1 - so user can click in the manager preview on a button and get things done. There is an example i official-storybook where a counter is being increased. Not in love with this.

[tmeasday]

I think maybe things are a little confused so let me (try) and be clear here:

There is no generic story state. Args is used only for storing inputs to stories. As the README says:

Note that arg values are passed directly to a story -- you should only store the actual value that the story needs to render in the property. If you need more complex information supporting that, use parameters or addon state.

To your points:

context.args does not work for me and will create incompatible stories. Here is what I would like to have:

export const story = props => <Button {...props} />;

That's what the parameters.options.passArgsFirst option gives you: https://github.com/storybookjs/storybook/blob/448a65daa60d8660ed42696e03623b21188e152c/lib/client-api/README.md#using-args-in-a-story

Is that not what you want?

-- adding the 'smart-controls' to prop types - no, I did no ask for such thang. Maybe a misunderstanding - I did add a type field tp propDefs since it was//is being used mostly for display and the actual type information was lost.

I know, this was something @shilman and I decided made sense given the work he is doing on refactoring the "arg type" (prop table) inference code and this work. So to be clear:

In your code:

1. Both the prop table, and your smart "controls enhancer" called out to the addon-docs code to produce the "arg type" information for the story.

2. Your smart controls enhancer wrote that type information to the store in the "controls" field.

3. That type information was used to render controls and initialize initial control values.

In my design:

1. The docs addon adds a "parameter enhancer" which calls the same code and adds the same type information to parameters.argsTypes.

2. The prop table then just reads off that parameter in the context when rendering -- simple.

3. The store will use parameters.argsTypes to initialize initial arg values.

4. Any code that works with args can similarly use theparameters.argsTypes to render controls.

So it's not really all that different it's just a reorganization.

-- resetValues - I dont need this anymore. It was initially in there as I thought it would help other implementations of knobs.
-- events like onClick - mostly for compatibility with knobs-1 - so user can click in the manager preview on a button and get things done. There is an example i official-storybook where a counter is being increased. Not in love with this.

OK, that's helpful. I'll not focus on this for now.

[atanasster]

Just to make sure - argTypes is simply my controls renamed - It is not a shared story state? If thats the case, great job.

parameters.options.passArgsFirst

Cool, i am very glad to see this one made it across.
It is my strong belief that this should be the default behavior and now is the only time to do in 6.0. Otherwise people will start creating stories with all sorts of incompatible code.
The alternate behavior(the one you currently list as the default) should probably be hidden or kept just for some weird addon decorators.

Your smart controls enhancer wrote that type information to the store in the "controls" field.

Not correct - no type information is written, its a set of brand new control fields that are created.

The prop table then just reads off that parameter in the context when rendering -- simple.

Not going to work as far as i understand (but also with all the new names i have a hard time to understand what is what so might be wrong) - this information is needed outside of prop tables as well.

In any case i am writing my own loader and will probably not need this part of the code as well, so its really up to you what you would like to do.

parameters.argsTypes

I am not the variable names police, but argTypes is not easy for me to understand what it does.

I strongly disagree with using the parameters name space. I thought we discussed this and dont want to repeat it but here it is just for completeness

 MyStory.story = {
  controls: {}
}

So it's not really all that different it's just a reorganization.

The reorganization looks great, for the variables changing names - it breaks my integration code, not sure if thats important to you.

onClick - i kinda wanted to have as much compatibility as possible with knobs. Mostly as a homage as its one of the most used and liked add-ons.

[atanasster]

A few references as to why I finally settled on naming them controls
-blocks-ui, called controls are pretty much the same as what knobs (or my controls) could be described
https://blocks-ui.com/docs/controls

-Framer - called ‘property controls’ are exactly the same as my controls. In this case both properties and controls are ok. https://www.framer.com/api/property-controls/

The reason i am using outside references is that I think its best to use variable names thta users are already used to.

I first used the name properties but it could have been confused with the prop tables properties and switched to controls.

In any case, its a breaking change for my code and it feels a bit disrespectful to my work but also i dont really see argTypes as a good, suggestive name.

[tmeasday]

Regarding the naming:

• "args" are the names of the set of "inputs" (and things like actions and outputs and contexts) to a story. This is the thing the the user is thinking about when writing stories. We would have called them "props" but that would have been confusing for non-react users I think.

• "arg types" is just the type information about args. Analogous to "prop types".

• "controls" is the name of the UI elements that the user users to affect arg values. So your addon would still make sense to be called addon controls etc. I think you are right that it maps well to existing concepts.

Happy to debate this further but this feels better in a world where args are about more than controls -- you can use args without controls, and you can use affect args with things that aren't controls (like actions and context).

Some details:

Your smart controls enhancer wrote that type information to the store in the "controls" field.

Not correct - no type information is written, its a set of brand new control fields that are created.

Ok, that's fair I was being a bit hand-wavey here. But the control field information is just a different expression of the type information produced by addon-docs, right? So in a sense it is that type information.

It is my strong belief that this should be the default behavior and now is the only time to do in 6.0. Otherwise people will start creating stories with all sorts of incompatible code.

I mean people have been doing this for many many years all the way up 5.3. That's why we'd want to bring this in slowly and make sure people have an opt-in/out. But depending on the beta process etc, I could see us defaulting to true and making it an opt out. Let's see how people take to it.

I strongly disagree with using the parameters name space. I thought we discussed this and dont want to repeat it but here it is just for completeness

I get it. But the idea is to not add new features to CSF until we've had a chance to actually see how they work in the wild and get feedback from real use cases. Experience tells us that until we release an actual version we won't get the kind of complete feedback that comes from wide real world use (sadly).

The reorganization looks great, for the variables changing names - it breaks my integration code, not sure if thats important to you.

Do you mean addControls? I don't think we need that if we put the configuration in parameters. And if we end up adding it to CSF, I think we simply change preview.js to export an object ({parameters: .., decorators: ..., /* anything new */ }) rather than adding a new export to every framework, like we talked about the other day.

I appreciate it is annoying as you already did that (adding addControls).

[atanasster]

Happy to debate this further but this feels better in a world where args are about more than controls -- you can use args without controls, and you can use affect args with things that aren't controls (like actions and context).

I like it when we can actually debate the ideas, so thanks for that.

In Framer they are exactly describing the properties and using the word controls - so the definition became the same as the usage.

I agree the best name was properties (not prop as its just react) - but my concern was confusion with the prop tables where properties is also used.

In conclusion - I have already done thousands of tests, I have/am/and will be writing hundreds of stories by the time your code is ready. So your argument (pun intended:)) is not strong enough to make me spend a lot of time re-testing and writing the stories anew.
So I stay with controls. I think you should also switch, but its your decision .

Ok, that's fair I was being a bit hand-wavey here. But the control field information is just a different expression of the type information produced by addon-docs, right? So in a sense it is that type information.

I am lost then what you are referring to. I thought you mean my enhanceControls which is now renamed to something?

But this one is definitely not related to type information - yes, smart-controls uses it in this way but it can be anything else - lets say create new controls dynamically (if the user has enter a 'color' field, also add a 'size' field or anything else)

The only thing related to type information is that I extracted the collection of type information and made it available to outside packages. So smart-controls was using this export.

Or do you mean the enhancement of the Props table (adding a column to it) from a field called propsTable- not related at all with smart-controls, works with all controls.

  parameters: {
  docs: {
    addons : {
      propsTable: fnEnhancePropsTable
    }
  } 
}

I mean people have been doing this for many many years all the way up 5.3.

If you mean export const myStory = (someContext) => <Button />, actually they mostly haven't. So I can re-iterate - now is the time to make this act the right way as default.

I get it. But the idea is to not add new features to CSF until we've had a chance to actually see how they work in the wild and get feedback from real use cases. Experience tells us that until we release an actual version we won't get the kind of complete feedback that comes from wide real world use (sadly).

Well - experience tells us that those two are incompatible and you cant switch once users start using one of the formats.

The reorganization looks great, for the variables changing names - it breaks my integration code, not sure if thats important to you.
Do you mean addControls?

No, that was the least of my worries. I did mention to @shilman that I am ok users importing { addControls } from 'client-lib'; instead of framework. Or better yet - re-design that code so there is no limit to just lump everything into parameters.

But my biggest concerns are
controls -> argType
enhanceControls -> something else
... and pretty much everything else.

I detailed above how this is impacting my work and really for no good reason.

[atanasster]

The only thing related to type information is that I extracted the collection of type information and made it available to outside packages. So smart-controls was using this export.

This one I added a field type to propDef.type as the type info was lost with some prop readers.

I think @shilman is now renaming it to raw :) But its not stored in controls at all - its still part of the prop definitions.

[atanasster]

Happy to debate this further but this feels better in a world where args are about more than controls -- you can use args without controls, and you can use affect args with things that aren't controls (like actions and context).

Ooops - completely missed the part that you say you can add context to the args. I really thought that we had closed that discussion - controls and context/state/ anything else should be kept separate.
If you are still planing on lumping them together, then thats an even bigger issue than just using a name that breaks all code.

Btw, i forgot to remind you that you are also breaking my documentation by renaming my variables. @shilman should have a link for it.

[stale]

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]

Hey there, it's me again! I am going close this issue to help our maintainers focus on the current development roadmap instead. If the issue mentioned is still a concern, please open a new ticket and mention this old one. Cheers and thanks for using Storybook!