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

Docs doesn't have diff support #16080

Open
rajzik opened this issue Sep 16, 2021 · 12 comments
Open

Docs doesn't have diff support #16080

rajzik opened this issue Sep 16, 2021 · 12 comments

Comments

@rajzik
Copy link

rajzik commented Sep 16, 2021

Describe the bug

Addon docs doesn't support diff.

- myChange
+ myOtherChange

Current state:
image

To Reproduce

https://github.com/rajzik/sb-repro

Other deployed SB can be found: https://next-storybook.oriflame.com/?path=/story/general-changelog--page

System

Environment Info:

  System:
    OS: Windows 10 10.0.22000
    CPU: (24) x64 AMD Ryzen 9 5900X 12-Core Processor
  Binaries:
    Node: 14.17.6 - C:\Program Files\nodejs\node.EXE
    Yarn: 3.0.2 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 7.21.1 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Spartan (44.22000.120.0), Chromium (93.0.961.47)
  npmPackages:
    @storybook/addon-actions: ^6.4.0-alpha.37 => 6.4.0-alpha.37
    @storybook/addon-docs: ^6.4.0-alpha.37 => 6.4.0-alpha.37
    @storybook/addon-essentials: ^6.4.0-alpha.37 => 6.4.0-alpha.37
    @storybook/addon-links: ^6.4.0-alpha.37 => 6.4.0-alpha.37
    @storybook/react: ^6.4.0-alpha.37 => 6.4.0-alpha.37

Additional context

It would be great to have at least some support for diff.

@jonniebigodes
Copy link
Contributor

Hi @rajzik , that seems a bit strange. Did you try applying what is documented here and see if it works on your case?

To give you a bit more context, out of the box you get syntax highlighting for some languages (e.g., JS, TS, Graphql). To enable diff you have to configure the syntax highlighter to enable in your case diff.

I tried it with your reproduction and it's working, here's what i did:

  • Cloned your repo
  • Installed the dependencies
  • Updated the story to the following:
import { Meta, Description } from "@storybook/addon-docs";
import Readme from "../README.md";
import { Prism as SyntaxHighlighter } from "react-syntax-highlighter";
import { dark } from "react-syntax-highlighter/dist/esm/styles/prism";

<Meta title="General/Readme" />

<Description>{Readme}</Description>

export const Component = () => {
  return <SyntaxHighlighter style={dark} />;
};
  • Ran yarn storybook and it yielded the following:
    diff-running-storybook-optimized

  • I've checked with a production build of Storybook with yarn build-storybook && npx http-server storybook-static, to create the production build of Storybook and running HTTP server locally to preview the build and it yielded the same result.

Feel free to provide feedback, so that we close this issue, or continue to work on it until we find a solution for your issue.

Stay safe

@rajzik
Copy link
Author

rajzik commented Sep 17, 2021

@jonniebigodes it works. But is it possible to set syntax highlighter globally? I had to change quite a few files to get it done.

@jonniebigodes
Copy link
Contributor

@rajzik sorry for the delayed response. But i was otherwise engaged ! To the best of my knowledge you'll have to use the workaround that is currently documented. I'm aware that is quite cumbersome if you have a growing Storybook that uses MDX extensively. But to give you some context on this issue, in a previous Storybook release it was decided to remove some of the supported syntaxes in favor of performance. It might not be apparent to most users, but if you or any other user was working on a huge Storybook it would start to take a toll either in development mode or production mode.

If you would be willing to work on this with us and provide a way that doesn't affect performance and provide you and the rest of the community with a solution we'd love to follow up with on it.

If you're interested, check out our contribution documentation.

Looking forward for your pull request.

Have a great day!

Stay safe

@timbomckay
Copy link

@jonniebigodes I've seen you import the dark style in a couple issue threads but the screen shot never shows the dark styles.

According to react-syntax-highlighter's demo the dark styles for prism should look like this:

image

I'm currently v6.3.12 and it seems like no matter what I pull in the styles look the same.

import SyntaxHighlighter from 'react-syntax-highlighter';
import { atomOneDarkReasonable } from 'react-syntax-highlighter/dist/esm/styles/hljs';

export const Component = () => {
  return (<SyntaxHighlighter style={atomOneDarkReasonable} />);
};

"scss
$font-stack: Helvetica, sans-serif;
$primary-color: #333;

body {
  font: 100% $font-stack;
  color: $primary-color;
}
"

No matter which style I pull in it always renders the same:

image

Which looks like what you're showing above, not matter if it's Prism or HLJS. Is this a bug or has this been addressed in a newer version or is this the expected behavior, to force the one style? It seems like all this code does is restores all the other languages.

Thanks

@jonniebigodes
Copy link
Contributor

@timbomckay the usage of the dark theme was for experimental purposes only and when answering the issue I left it unintentionally. I've checked on my end and was able to confirm that no matter the styles applied they aren't loading, with the exception of dark. For instance, if I change a snippet to:

"diff dark
- import { OldComponent } from 'my-package'
+ import { NewComponent } from 'my-package'

The usage of ("") is to prevent the snippet from being processed.

It yields the following:
darkmode-syntax-highlight

I've tested this on a reproduction running Storybook 6.3 and bumped the reproduction to the latest version and same results, asides from the dark none other worked.

@GuilleDF
Copy link
Contributor

Hi all! Not sure if it's my particular setup, but this workaround doesn't seem to be working for me. As far as I can tell, react-syntax-highlighter seems to be bundled into the @storybook/components package, so importing it and adding an export (or calling registerLanguage, like suggested in the docs) has no effect.

My storybook version is 6.5.0-alpha.49

@GuilleDF
Copy link
Contributor

Update: this was working for me on version 6.5.8 but this PR #18425 changed react-syntax-highlighter from a dependency to a bundled package.

This makes the docs in this page https://storybook.js.org/docs/react/writing-docs/mdx#syntax-highlighting not work on 6.5.10 😕

@jimmynotjim
Copy link

This is happening to me as well. I've lost all highlighting not automatically included, including both diff and scss. I've tried using both the single page and global settings provided in the syntax highlighting docs as well as including react-syntax-highlighter within my own dependencies. Any clues on what else I should try?

@GuilleDF
Copy link
Contributor

GuilleDF commented Sep 1, 2022

@jimmynotjim My current workaround is to pin the version to 6.5.9 in the package.json, where it still works

-"@storybook/addon-essentials": "^6.5.0",
+"@storybook/addon-essentials": "6.5.9",
-"@storybook/core-common": "^6.5.0",
+"@storybook/core-common": "6.5.9",
-"@storybook/react": "^6.5.0",
+"@storybook/react": "6.5.9",

(For all @storybook/* packages I have)

@jameschensmith
Copy link

I'm experiencing the same issue as the most recent replies. I tried following the docs on adding a new language to highlight, but they were probably written before the dependency was bundled, also mentioned above. I thought it was because of Vite, but it appears to possibly be due to the bundling of react-syntax-highlighter. I'm surprised there isn't more users replying to this issue. Is there a solution to address this without having to pin the dependencies to a past version?

@jonniebigodes, you had originally added the "has workaround" tag to this issue. I don't think it's applicable anymore, as pinning to an old version isn't a long-term workaround. Would you be able to look into this ticket again when you get a chance, please?

@Christo44
Copy link

Using as interim seems to work.

<Source language="css" dark format={false} code={ your code here } />

@shilman shilman removed the PN label Apr 24, 2023
@razlifshitz
Copy link

Any updates? We use Storybook v7 and we're having this issue as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Empathy Backlog
Development

No branches or pull requests

9 participants