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

Cannot use symbol as story argument property #14055

Closed
JesusTheHun opened this issue Feb 25, 2021 · 5 comments
Closed

Cannot use symbol as story argument property #14055

JesusTheHun opened this issue Feb 25, 2021 · 5 comments

Comments

@JesusTheHun
Copy link

JesusTheHun commented Feb 25, 2021

Describe the bug

I was doing some dark magic to be able to test connected component and I wanted to use a symbol into the template arguments but it turns out they are stripped away :

Use case

export default {
  title: 'Acme/Pages/Settings/Invitations',
  component: InvitationsPage,
  decorators: [withDisconnectedStore(authenticatedCustomer)],
} as Meta;

// Then use
export const Default = Template.bind({});
Default.args = { [ConnectedStorePropsKey]: myStoreWhenPopulated };

export const Empty = Template.bind({});
Empty.args = { [ConnectedStorePropsKey]: myStoreWhenEmpty };

export const Loading = Template.bind({});
Loading.args = { [ConnectedStorePropsKey]: myStoreWhenLoading };

Description

export const ConnectedStorePropsKey = '$connectedStore'; // Here lies the issue
export type HaveConnectedProps = { [ConnectedStorePropsKey]: PreloadedState<RootState>}
export type WithConnectedProps<T> = T & HaveConnectedProps;

export const withDisconnectedStore = <T extends unknown>(sharedStoreState: PreloadedState<RootState>) => (story: () => StoryFnReactReturnType) => {
  const component = story();
  const props = component.props as HaveConnectedProps;
  const storeProps = props[ConnectedStorePropsKey];
  const initialState = _.merge({}, sharedStoreState, storeProps);
  const storeElements = buildDisconnectedStore(initialState);
  return <ProviderWrapper store={storeElements.store} history={storeElements.history}>{component}</ProviderWrapper>;
}

As you can read the const ConnectedStorePropsKey is currently a string: that's my workaround.
Initially I used a symbol (that was exported to be use in the stories) but it turns out it is striped away and the const component never carries it, Object.getOwnPropertySymbols(props).length === 0

Is there any particular reason why and is it a desired behavior ?

@phated
Copy link
Contributor

phated commented Feb 25, 2021

Args must be serializable to JSON - this is done with https://github.com/storybookjs/telejson, but that only supports symbols as values in args. I'm not sure non-string keys are something we'd want to support, but I'll ask around.

@ghengeveld
Copy link
Member

ghengeveld commented Feb 26, 2021

This will be addressed in #13888

The gist is we'll introduce a kind of "select" control for which keys are used as arg (which gets serialized/deserialized) rather than the value. That way the value can be anything, because it will stay within the scope of the preview and not need to be sent to the manager over the channel (which uses telejson).

Closing as duplicate.

@JesusTheHun
Copy link
Author

JesusTheHun commented Feb 26, 2021

Symbol as key is a pretty "common" thing. By common I mean it's used a lot in TypeScript for advanced types like nominal types aka branding. It also allow some really cool architecture.
The issue I can see with telejson and Symbol is that unless they are globally registered symbols you cannot recreate them, you have to get their reference somehow, usually you just import them but I don't see how you can do that with telejson. I mean even as values, your test does not reflect the real usage of symbol as they are not compared directly but stringified.
Using Symbol.for(key) and Symbol.keyFor(symbol) you could get the real reference to a symbol from the global registry. It's only a partial support but it's a true/strict one.
I just submitted a PR on telejson for global Symbol support.

@ghengeveld
Copy link
Member

@JesusTheHun it would be great if you could get Symbols to work properly in telejson. Another issue however is serialization to the URL, which only allows [a-z0-9 _-]* alongside !null and !undefined. That's why we're investing in a workaround where you use a simple string as the arg value, and apply a mapping to convert that to the actual (complex) value. That complex value is never sent over the channel, so it doesn't have to be serialized with telejson.

@JesusTheHun
Copy link
Author

@ghengeveld I've created a PR in telejson to truly support global symbols but it is simply impossible to have local symbols to be serialized, it's a JS limitation that is also a feature.

Now regarding the serialization to the url, I don't know where you need that but it's very likely a bad solution. You could base64'ify the content to pass the data but then you run into browser url size limitation of 2048 chars.

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