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

withActions: CustomEvent detail missing #8544

Closed
rdm opened this issue Oct 23, 2019 · 25 comments
Closed

withActions: CustomEvent detail missing #8544

rdm opened this issue Oct 23, 2019 · 25 comments

Comments

@rdm
Copy link

rdm commented Oct 23, 2019

Is your feature request related to a problem? Please describe.
I had originally filed this as a bug report, but belatedly decided that it's technically a feature request.

Basically, though, when I am building a WebComponent which generates a CustomEvent, I want to be sure that the CustomEvent is being generated properly.

Describe the solution you'd like
withActions (from @storybook/addon-actions) should include in the log the detail property of an event it receives if the event is a CustomEvent (or there should be some other documented way for stories to listen for a CustomEvent and see its detail property in the log).

Describe alternatives you've considered
(using html from lit element and render from lit html), I render:
<button type="button" @click="${this.publishtest}">publish</button>

And, I add a publish method on that class:

publishtest() {
  this.dispatchEvent(new CustomEvent('pubexample', {detail: {a: 1, b: 2}, bubbles: true}));
}

And, in my stories I include:
.addDecorator(withActions('pubexample'))

I click the button, and something representing my example event shows up in the console log, but the detail property appears to be missing (or at least is not accessible from the console.log in chrome).

Are you able to assist bring the feature to reality?
I don't know - I'll try implementing this tomorrow and submit a pull request if I can make this work.

Additional context
Possibly relevant is that the logged message has a data property which has an args property which is an array of one element which is seems to be my CustomEvent but the only visible property of that item is {isTrusted: false}. Meanwhile, if I save that CustomEvent globally, and display it in the log, I see something completely different (with about 16 properties, but Object.keys() on that object only gets me isTrusted and _constructor-name_ -- so a part of the issue here is simply that chrome doesn't let me see inherited properties in the context of the logged message, perhaps because the CustomEvent is deeply nested in the message).

@sinharaksh1t
Copy link

sinharaksh1t commented Oct 25, 2019

Just curious - on this line:

<button type="button" @click="${this.publishtest}">publish</button>

Did you mean onClick instead of @click? Or is this @click something that I don't know about?

@rdm
Copy link
Author

rdm commented Oct 25, 2019

The @click mechanism is slightly different from onClick -- see, for example https://lit-element.polymer-project.org/guide/events

[And, on the positive side, despite a sordid history of CustomEvent problems with detail, in this context detail is not being lost -- it's just not visible as it's logged here. (But I haven't come up with a better alternative for logging, yet, either.)]

@stale
Copy link

stale bot commented Nov 15, 2019

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 Nov 15, 2019
@stale
Copy link

stale bot commented Dec 15, 2019

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!

@stale stale bot closed this as completed Dec 15, 2019
@dmartinjs
Copy link
Contributor

I have also this issue, can you reopen ?

@shilman shilman reopened this Jan 9, 2020
@stale stale bot removed the inactive label Jan 9, 2020
@stale
Copy link

stale bot commented Jan 30, 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 Jan 30, 2020
@rdm
Copy link
Author

rdm commented Jan 30, 2020

Given the current state of things, this is likely to take much longer than 30 days to address.

@stale stale bot removed the inactive label Jan 30, 2020
@shilman
Copy link
Member

shilman commented Jan 31, 2020

@rdm would you like to take a crack at it?

@rdm
Copy link
Author

rdm commented Jan 31, 2020

@shilman there are still too many issues which I do not comprehend, here. I think I'd botch the job.

Still... maybe I should try -- a botched job might at least be a step in the right direction...

Edit: currently, storybook tests do not succeed for me, on master because of some insufficient color contrast issues. That's from the puppeteer report -- I'm getting a variety of additional require "package not installed as a dependency" failures when I try testing locally...

@stale
Copy link

stale bot commented Feb 21, 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
Copy link

stale bot commented Mar 23, 2020

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!

@stale stale bot closed this as completed Mar 23, 2020
@shilman
Copy link
Member

shilman commented Mar 23, 2020

cc @jonspalmer

@urish
Copy link
Contributor

urish commented Mar 29, 2020

Seems like this issue is coming from react-inspector.

I managed to reproduce it there as follow:

  1. Go to the react-inspector playground
  2. Open the devtools console and type window.eval = () => new CustomEvent('test', {detail: 123})
  3. Type anything in the code editor on the left (doesn't matter what, as long as you change the value), and you will see that the inspector on the right shows an Object with a single isTrusted property, and no mention of the details property, similar to what happens in storybook:

image

At the same time, running console.log(new CustomEvent('test', {detail: 123})) in chrome devtools shows a bunch of properties, including detail:

image

I hope that this is helpful!

@jeremie-qlf
Copy link

jeremie-qlf commented Jun 7, 2020

After doing a few tries, I discovered that almost all the "properties" of the CustomEvent are actually class getters, the only property is isTrusted.

It means, that if you try to duplicate or serialize a CustomEvent, you will only get the property isTrusted since class getters are not considered as properties.

Examples:

  • Object.assign({}, new CustomEvent('test', {detail: 123})) // => { isTrusted: false }
  • JSON.stringify(new CustomEvent('test', {detail: 123})) // => {"isTrusted":false}

I assume that react-inspector is doing a serialization or a copy of the original event somewhere before to render it. Resulting in the loss of the other "properties".

I managed to workaround that issue by adding a custom decorator to the addons-actions like this:

// .storybook/helpers/actions.js
import { decorate } from '@storybook/addon-actions';

const eventProperties = [
  'bubbles',
  'cancelBubble',
  'cancelable',
  'composed',
  'currentTarget',
  'defaultPrevented',
  'detail',
  'eventPhase',
  'isTrusted',
  'path',
  'returnValue',
  'srcElement',
  'target',
  'timeStamp',
  'type',
];

const cloneEventObj = (eventObj, overrideObj = {}) => {
  class EventCloneFactory {
    constructor (override){
      for(const prop of eventProperties){
        this[prop] = eventObj[prop];
      }

      for(const prop in override){
        this[prop] = override[prop];
      }
    }
   }
   EventCloneFactory.prototype.constructor = eventObj.constructor;
   return new EventCloneFactory(overrideObj);
}

const uniqueId = (eventName) => `${eventName}-${(new Date()).getTime()}`;

export const customEvent = decorate([args => {
  const originalEvent = args[0];
  const ev = cloneEventObj(originalEvent, {
    id: uniqueId(originalEvent.type),
  });
  return [ev];
}]);
// story/test.stories.js
import { customEvent } from '../.storybook/helpers/actions';

export default {
  title: 'Test',
  decorators: [
      customEvent.withActions('test'),
  ],
};

...

There's probably a better way to do that, but at least it works 🤷‍♂️

@dmartinjs
Copy link
Contributor

thanks @jeremiej-q , it works very well 👏

@AGrabovajFitA
Copy link

Here is a solution to this problem: https://stackoverflow.com/questions/13333683/javascript-customevent-detail-not-getting-passed

@dutscher
Copy link

dutscher commented Dec 10, 2020

@shilman @ndelangen the problem in here is the communication between manager and preview:
https://github.com/storybookjs/storybook/blob/next/lib/channel-postmessage/src/index.ts#L77

look at the console screenshot:
image

and the result in the action console is:
image

the telejson package get the constructor_name and JSON.stringify did the rest and only return "isTrusted"
https://github.com/storybookjs/telejson/blob/master/src/index.ts#L209
as you can see at @jeremiej-q posts.

i think we can reopen this issue!

edit:
@jeremiej-q your fix didn't work in storybook 6
for telesync i found a solution to get all getter values from event into actions:
https://github.com/storybookjs/telejson/blob/master/src/index.ts#L209

   Object.assign(value, {
       '_constructor-name_': value.constructor.name,
       eventData: JSON.parse(JSON.stringify(value, Object.keys(value.constructor.prototype))),
       details: value.detail,
   });

will show like this:
image

cheers

@shilman
Copy link
Member

shilman commented Dec 11, 2020

@dutscher what are you recommending here? ☝️

@dutscher
Copy link

@shilman
an update for telesync is necessary
https://github.com/storybookjs/telejson/blob/master/src/index.ts#L209 in this line we need

const constructorName = value.constructor.name;
Object.assign(value, {
  '_constructor-name_': constructorName,
  ...(constructorName.toLowerCase().includes('event') ? {
    eventData: JSON.parse(JSON.stringify(value, Object.keys(value.constructor.prototype))),
    details: value.detail,
  } : {}),
});

then we get the result as my snapshots above shown.

cheers

@shilman shilman reopened this Dec 20, 2020
@shilman
Copy link
Member

shilman commented Dec 20, 2020

@ndelangen is this an uncontroversial fix that you'd approve? ☝️

@gaetanmaisse
Copy link
Member

💪🏻 @dutscher, facing the same issue so I would be happy to help on this one. @ndelangen do you think it would be ok to apply the changes suggested above (#8544 (comment))?

Maybe we need to be stricter on the constructor name check in order to not end in that case for any *Event* classes and explicitly list some specific class name. Otherwise, we can keep the check as it and check that value.detail is defined before setting it. Feels free to ping me on Discord to discuss that.

@stale stale bot removed the inactive label Feb 7, 2021
@jesperp
Copy link

jesperp commented Mar 28, 2021

FYI: This error also happens with Svelte!

@dutscher
Copy link

i use a workarround until the fix is inside of telejson:

var uniqueId = function(eventName) {
        return eventName + '-' + (new Date()).getTime();
      };

__STORYBOOK_ADDONS.getChannel()
              .emit('storybook/actions/action-event', {
                count: 0,
                data: {
                  name: eventName,
                  args: event.detail,
                },
                id: uniqueId(eventName),
                options: {
                  depth: 15,
                  clearOnStoryChange: true,
                  limit: 50,
                  allowFunction: false,
                }
              });

cheers

@shilman
Copy link
Member

shilman commented Apr 1, 2021

We want to address this in 6.3. If you want to contribute to Storybook, we'll prioritize PR review for any fixes here. And if you'd like any help getting started, please jump into the #support channel on our Discord: https://discord.gg/storybook

@shilman
Copy link
Member

shilman commented May 11, 2021

Gadzooks!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.3.0-alpha.23 containing PR #14879 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.

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

10 participants