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

Inability to escape slashes in story names #9761

Closed
adamdoyle opened this issue Feb 5, 2020 · 8 comments
Closed

Inability to escape slashes in story names #9761

adamdoyle opened this issue Feb 5, 2020 · 8 comments

Comments

@adamdoyle
Copy link
Contributor

Describe the bug
My team groups our stories by the scoped packages that contain the components under test/display; e.g.:

 @org/widgets
    <some-widget>
    <some-other-widget>
 @org/icons
    <some-icon>
    <some-other-icon>

  ...

  etc.

Previously the forward-slash in the scoped name wasn't an issue, since the separator character could be configured, but this configurability was deprecated in #8793.

The issue talked about providing an escape character, but it doesn't appear to ever get implemented.

To Reproduce
(1) Attempt to escape a forward-slash in a story name
(2) Observe that the splitting still happens

Expected behavior
At a minimum, I would at least expect to have the ability to escape the forward-slash.

Unfortunately, even if an escape character is added, it will mean I'll either have to (1) change my story definitions from something readable to something that's mangled with escape characters, or else (2) wrap each of my story name definitions in a function call [that would live in one of my libraries somewhere] that would replace @org/ with @org// (or whatever the escape character is).

Additional context
The reasons given for removing configurability were as follows:

  1. We need to carry the option around anywhere we want to split stories.
  2. It makes it hard to look at a kind name and understand the hierarchy simply
  3. In theory you could set a different separator for each story, although we don't actually support that

I'm not sold on the solution that was reached. Below are two alternative solutions that I think might have been better for all parties:

Alternate Solution 1

(1) Leave the API as-is, keeping the separator and root separator configurable
(2) Upon initial load of the configuration, lookup the user's custom separator preferences and "normalize" the story names by replacing the custom separators with "storybook-standard slash separators" [escaping the user's slashes in cases where the user's separator preference isn't a slash]
(3) Instead of passing around "raw story names" and "separator preferences" all over the place, only pass around the normalized story names that already have the separators replaced with slashes and already have any non-separator slashes escaped.

This way SB maintainers only have to deal with minimal separator-configuration overhead and it's all localized at the entrypoint. Users get clean story names that's free of escape characters and flexible enough to match their use-case-specific needs. SB maintainers get simpler/cleaner code without having to take away functionality that's been around for a long time.

Alternate Solution 2

Split story names once on first read. Instead of passing around the unprocessed names, pass around an array of pre-split (according to configurable separator preferences) story names.

@shilman
Copy link
Member

shilman commented Feb 5, 2020

cc @tmeasday

@tmeasday
Copy link
Member

tmeasday commented Feb 5, 2020

I take your point @adamdoyle, although the whole area is kind of complicated. @shilman and I agonised about this for ages :/

However, we are probably going to do a refactor of the story store for 6.0 that will present opportunities to revisit this. Perhaps the idea of splitting stories as they come into the store and only dealing with the array of pre-split "parts" is the way to go. I wouldn't want to reintroduce a root separator (then you have to also carry around whether or not the first part is a root, which is just a mess) but potentially having a path separator would be OK.

Will need to try and remember all the factors that went into the decisions.

@adamdoyle
Copy link
Contributor Author

Would it be possible to keep the separator configurability not fully removed (but maybe still deprecated) until that revisit happens? (for those of us with slashes and no mechanism for escaping them)

@tmeasday
Copy link
Member

tmeasday commented Feb 6, 2020

Well 6.0 will be the next release so we certainly won't be changing anything until then.

PS if others are relying on using / in group names, can they please +1 the original issue here so we get a feel for how many people this affects?

@stale stale bot added the inactive label Feb 27, 2020
@tmeasday tmeasday removed the inactive label Feb 28, 2020
@storybookjs storybookjs deleted a comment from stale bot Feb 28, 2020
@stale
Copy link

stale bot commented Mar 20, 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 Mar 20, 2020
@stale
Copy link

stale bot commented Apr 19, 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 Apr 19, 2020
@gossi
Copy link
Contributor

gossi commented May 16, 2020

Hack storybook like a pro, use a homoglyph: https://www.irongeek.com/homoglyph-attack-generator.php

Works like charm ;)

You might want use the - in your story name and then use the storySort to replace the - with your glyph. That keeps the URL of your story "clean".

@sushitommy
Copy link

Or use ⁄ html code, works just as fine

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

5 participants