storybookjs / storybook

Storybook is the industry standard workshop for building, documenting, and testing UI components in isolation
https://storybook.js.org
MIT License
84.54k stars 9.3k forks source link

DocsPage code preview indentation is wrong #8078

Closed danielcolinjames closed 1 year ago

danielcolinjames commented 5 years ago

Issuehunt badges

Describe the bug When creating stories for a DocsPage with an MDX file, the code preview underneath the story seems to have the wrong indentation, even though it's correct in the file.

To Reproduce Steps to reproduce the behavior:

  1. Create a Component.stories.mdx file
  2. Output a <Story name="default"><Preview><Component /></Preview></Story>
  3. Go to DocsPage for that component
  4. Click "Code" button to bring up code preview
  5. See wrong indentation for code preview

Expected behavior The indentation (and line breaks) would be identical to how it shows up in the MDX file.

Screenshots The code:

and how it shows up in the DocsPage <Preview>:

Code snippets If applicable, add code samples to help explain your problem.

System: Environment Info:

System: OS: macOS 10.14.6 CPU: (8) x64 Intel(R) Core(TM) i7-4870HQ CPU @ 2.50GHz Binaries: Node: 12.6.0 - /usr/local/bin/node Yarn: 1.17.3 - /usr/local/bin/yarn npm: 6.9.0 - /usr/local/bin/npm Browsers: Chrome: 76.0.3809.87 Firefox: 68.0.1 Safari: 12.1.2 npmPackages: @storybook/addon-actions: ^5.2.0-rc.10 => 5.2.0-rc.11 @storybook/addon-docs: ^5.2.0-rc.11 => 5.2.0-rc.11 @storybook/addon-links: ^5.2.0-rc.10 => 5.2.0-rc.11 @storybook/addons: ^5.2.0-rc.10 => 5.2.0-rc.11 @storybook/react: ^5.2.0-rc.10 => 5.2.0-rc.11

Additional context The same exact indentation abnormality (2nd through 2nd last lines are indented one tab too far left, and last line indented two tabs too far right) seems to happen for all my DocsPage code previews, so I figured it's either a configuration issue somewhere on my end, or a bug with the DocsPage code preview.

Thanks!


IssueHunt Summary ### Backers (Total: $20.00) * [kylemh kylemh](https://issuehunt.io/u/kylemh) ($20.00) #### [Become a backer now!](https://issuehunt.io/r/storybookjs/storybook/issues/8078) #### [Or submit a pull request to get the deposits!](https://issuehunt.io/r/storybookjs/storybook/issues/8078) ### Tips * Checkout the [Issuehunt explorer](https://issuehunt.io/r/storybookjs/storybook/) to discover more funded issues. * Need some help from other developers? [Add your repositories](https://issuehunt.io/r/new) on IssueHunt to raise funds.
stale[bot] commented 5 years ago

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!

mbbillz commented 4 years ago

Having the same issue, looks especially strange with render prop components... 😆 Screenshot 2019-11-19 at 11 46 01

shilman commented 4 years ago

@atanasster can you help out here?

atanasster commented 4 years ago

can someone please paste source code in text form to reproduce the issue instead of pics

shilman commented 4 years ago

@atanasster i think we have plenty of examples in the monorepo already, e.g.

https://storybook.js.org/docs/basics/writing-stories/#loading-stories

https://storybookjs-next.now.sh/official-storybook/?path=/docs/addons-a11y-form--without-label

atanasster commented 4 years ago

Thanks

shilman commented 4 years ago

Whoopee!! I just released https://github.com/storybookjs/storybook/releases/tag/v5.3.7 containing PR #9513 that references this issue. Upgrade today to try it out!

Closing this issue. Please re-open if you think there's still more to do.

mrtraser commented 4 years ago

Indentation still wrong for *.mdx files in storybook v5.3.8

Source:

Screenshot 2020-01-22 at 17 29 38

Result: Screenshot 2020-01-22 at 17 29 22

shilman commented 4 years ago

Boo-yah!! I just released https://github.com/storybookjs/storybook/releases/tag/v5.3.9 containing PR #9609 that references this issue. Upgrade today to try it out!

Closing this issue. Please re-open if you think there's still more to do.

ylacaute commented 4 years ago

The problem still exists in 5.3.9.

This code:

<div
  prop1="prop stay inline"
  prop2="prop stay inline"
  content2={{
    type: "indent ignored",
    value: "indent ignored",
    obj: {
      type2: "indent accepted from here"
    }
  }}
/>

is transformed to:

<div prop1="prop stay inline" prop2="prop stay inline" content2={{
type: "indent ignored",
value: "indent ignored",
obj: {
  type2: "indent accepted from here"
}
}} />
sezeregrek commented 4 years ago

Create the file .storybook/preview-head.html which includes the following style block.

<style> pre code { tab-size: 2 !important; } </style>

AnnyCaroline commented 4 years ago

I tried the @sezeregrek workaround, but I'm still getting indentation error in this case when I use MDX:

<Preview>
  <Story name="Single">
    {() => {
        const [isActive, setActive] = useSingle(1);

        return (
            <Accordion>
                {items.map((item, idx) => {
                    const active = isActive(idx)
                    return (
                        <div key={item.id}>
                            {item.title}
                        </div>
                    )
                })}
            </Accordion>
        )
    }}
  </Story>
</Preview>

image

If I define the same code using CSF, it works fine.

hasparus commented 4 years ago

I'd like to take this one. I did some quick research.

I've added a repro story (picture 1) to get the most hideous formatting. You can notice in the picture 2, that the code is altered not only in whitespace. It has also one pair of parentheses less.

I logged the source around addons/docs/src/blocks/Source.tsx#L45. We can already see the change in formatting there, this is before the Source from the components package renders.

I don't know yet where docs.source.code is assigned so I'll need to debug a bit.

@shilman Would you prefer to leave the story code as written or format it with prettier/standalone? Or maybe make it configurable in Source and Preview props? (There is a format prop in Source already, but it's only calling dedent.)

Formatting would also be useful for cases like "Counter w/ Code" story. It's 80 chars long, but it wraps in the docs page. (pictures 1 and 2)

Pictures

1. input

image

2. output

image

shilman commented 4 years ago

@hasparus Awesome, I'm so happy you're taking this. I think formatting the code with prettier is the way to go -- the user should have the option of auto-formatting or zero formatting, which I believe is already the case, just the auto-formatting logic is insufficient for most cases. I believe that's happening in syntaxhighlighter.tsx inside lib/components cc @ndelangen

hasparus commented 4 years ago

yup, this is the current formatter.

import memoize from 'memoizerific';
import dedent from 'ts-dedent';

export const formatter = memoize(2)((code: string) => dedent(code));

I like the code a lot, but dedent just isn't the big gun we need.


Okay, the work can be split in two parts.

1. if format is false, preserve user's source code as written

I think zero formatting option is broken ATM in MDX stories @shilman. The parentheses are removed, newlines are added. If it were only spaces, it could be some dedent issue or CSS tab-size problem.

The source code of the story is altered before getting to Source component. I think we should detect why it happens and preserve user's formatting if possible.

This fixes the bottom example in picture 2 above and leaves the first preview source ("Counter w/ Code") as is.

2. if format is true format with Prettier

This improves the case in "Counter w/ Code" story and "repairs" the alteration which is root cause of this issue if we force format == true in MDX stories.

I consider both the fix (1) and the feature (2) worth implementing.

caspardue commented 4 years ago

I've made the following patch-package until the correct fix is done. It works in my case (having prettier already installed). Use at your own risk. @storybook+components+5.3.18.patch

index 6a56b1b..457af0d 100644
--- a/node_modules/@storybook/components/dist/blocks/Preview.js
+++ b/node_modules/@storybook/components/dist/blocks/Preview.js
@@ -51,6 +51,17 @@ var _ActionBar = require("../ActionBar/ActionBar");

 var _Toolbar = require("./Toolbar");

+var prettier = require("prettier/standalone");
+var plugins = [require("prettier/parser-babel")];
+
+function funkyFormattingHack(code) {
+  var formatted = prettier.format(code, {plugins}).trim();
+    if(formatted.endsWith(';')) {
+      formatted = formatted.slice(0, -1);
+    }
+    return formatted;
+}
+
 function _getRequireWildcardCache() { if (typeof WeakMap !== "function") return null; var cache = new WeakMap(); _getRequireWildcardCache = function _getRequireWildcardCache() { return cache; }; return cache; }

 function _interopRequireWildcard(obj) { if (obj && obj.__esModule) { return obj; } if (obj === null || _typeof(obj) !== "object" && typeof obj !== "function") { return { "default": obj }; } var cache = _getRequireWildcardCache(); if (cache && cache.has(obj)) { return cache.get(obj); } var newObj = {}; var hasPropertyDescriptor = Object.defineProperty && Object.getOwnPropertyDescriptor; for (var key in obj) { if (Object.prototype.hasOwnProperty.call(obj, key)) { var desc = hasPropertyDescriptor ? Object.getOwnPropertyDescriptor(obj, key) : null; if (desc && (desc.get || desc.set)) { Object.defineProperty(newObj, key, desc); } else { newObj[key] = obj[key]; } } } newObj["default"] = obj; if (cache) { cache.set(obj, newObj); } return newObj; }
@@ -125,6 +136,9 @@ var PreviewContainer = _theming.styled.div(function (_ref3) {
 });

 var getSource = function getSource(withSource, expanded, setExpanded) {
+  if(withSource && withSource.code) {
+    withSource.code = funkyFormattingHack(withSource.code)
+  }
   switch (true) {
     case !!(withSource && withSource.error):
       {
diff --git a/node_modules/@storybook/components/dist/blocks/Source.js b/node_modules/@storybook/components/dist/blocks/Source.js
index ecfb5d7..eb3eea1 100644
--- a/node_modules/@storybook/components/dist/blocks/Source.js
+++ b/node_modules/@storybook/components/dist/blocks/Source.js
@@ -73,7 +73,7 @@ var Source = function Source(props) {
   var syntaxHighlighter = _react["default"].createElement(StyledSyntaxHighlighter, _extends({
     bordered: true,
     copyable: true,
-    format: format,
+    format: false,
     language: language,
     className: "docblock-source"
   }, rest), code);
avegancafe commented 4 years ago

Is there any work being done on this? I'd be happy to dig into it, though I don't have much experience with the storybook source code haha. Would love to have my source code unformatted (or even better, automatically formatted by prettier!)

shilman commented 4 years ago

@hasparus started a PR to use prettier but it stalled. I think the issue there is bundle size. I'd love some help to fix this. There's also another JS formatter used in storybook-addon-jsx that might be worth a look

avegancafe commented 4 years ago

Ahh I see @shilman , thanks for the update! Interesting, I'll check it out. I wonder if we could declare prettier as a peer dependency or something, and try to require it and fall back to the current version or something

hasparus commented 4 years ago

Hi @shilman, @keyboard-clacker. Yeah, the pandemic happened and life got a bit more complicated for me. I think I stopped on the problem with dynamic imports and code splitting. The components are in separate library, so Prettier can't be code splitted by webpack, and we should do something manually to lazily include prettier parsers. I dived into storybook's build system and didn't figure how to solve this.

ron0115 commented 4 years ago

yup, this is the current formatter.

import memoize from 'memoizerific';
import dedent from 'ts-dedent';

export const formatter = memoize(2)((code: string) => dedent(code));

I like the code a lot, but dedent just isn't the big gun we need.

Okay, the work can be split in two parts.

1. if format is false, preserve user's source code as written

I think zero formatting option is broken ATM in MDX stories @shilman. The parentheses are removed, newlines are added. If it were only spaces, it could be some dedent issue or CSS tab-size problem.

The source code of the story is altered before getting to Source component. I think we should detect why it happens and preserve user's formatting if possible.

This fixes the bottom example in picture 2 above and leaves the first preview source ("Counter w/ Code") as is.

2. if format is true format with Prettier

This improves the case in "Counter w/ Code" story and "repairs" the alteration which is root cause of this issue if we force format == true in MDX stories.

I consider both the fix (1) and the feature (2) worth implementing.

I set the format = false, fixed the indent problem, I also run well in my custom components below when I use react-syntax-highlighter originally.

import React from 'react'
import { Prism as SyntaxHighlighter } from 'react-syntax-highlighter'
import { atomDark } from 'react-syntax-highlighter/dist/esm/styles/prism'

const Highlighter = (props: any) => {
  const { language, children } = props
  return (
    <SyntaxHighlighter
      format={false}
      showLineNumbers
      startingLineNumber={0}
      useInlineStyles
      language={language}
      style={atomDark}
      customStyle={{ fontSize: '12px' }}
      wrapLines>
      {children}
    </SyntaxHighlighter>
  )
}
const CodePreview = () => {
  return <Highlighter language={'tsx'}>{String(require('!!raw-loader!@/Avatar').default)}</Highlighter>
}
export default CodePreview

so, just remove the formatter to fix this problem?

ittybittykitty commented 4 years ago

Hi, does anyone know if this bug would also extend to multiline template strings and return characters?

I have this in a story:

const twelveLineComment= `1. This
2. comment's
3. just
<b>4. twelve
5. lines
<i>6. long,
7. this</i></b>
8. comment's
9. just
<b>10. twelve
11. lines
12. long</b>`;

It renders like this:

  1. This 2. comment's 3. just 4. twelve 5. lines 6. long, 7. this 8. comment's 9. just 10. twelve 11. lines 12. long

And this: const comment = '***Pros***\r\nLots of room for books\r\nNice Color';

Should render like this: Pros Lots of room for books Nice Color

Renders like this: Pros Lots of room for books Nice Color

I don't know how to add a screen shot, but the line are being replaced with spaces.

ndelangen commented 4 years ago

That feels like a bit of a different issue?

This issue is about code-snippets getting incorrect indentation.

What you're describing is the rendering of html not respecting line-breaks. Which is a 'feature' of HTML I think?

Is this code in a mdx or md file? is this markdown format? Im not sure that would be valid markdown.

coreybruyere commented 4 years ago

I'm on the latest version of storybook and am still seeing incorrect indentation. Am I missing something?

Here's what my MDX looks like: Screen Shot 2020-08-25 at 9 29 10 AM

And here's the output: Screen Shot 2020-08-25 at 9 28 59 AM

shilman commented 4 years ago

@coreybruyere this issue is still open. PRs welcome!

github-actions[bot] commented 4 years ago

Automention: Hey @patricklafrance, you've been tagged! Can you give a hand here?

anna-tsukanova commented 4 years ago

Hello there! Stuck with the indentation issue. When using the control panel and the following code

const props = {
  phoneSize: 'm',
  icon: false,
  phoneNumber: '11111',
}

const Template = (args: any) => (
  <PhoneNumber {...args} />
)

export const Default = Template.bind({})
Default.args = props

I get the following preview: image

Has anyone come across a similar one? Thanks!

djizco commented 4 years ago

Hello there! Stuck with the indentation issue. When using the control panel and the following code

const props = {
  phoneSize: 'm',
  icon: false,
  phoneNumber: '11111',
}

const Template = (args: any) => (
  <PhoneNumber {...args} />
)

export const Default = Template.bind({})
Default.args = props

I get the following preview: image

Has anyone come across a similar one? Thanks!

I had this issue. I had a style that was over-riding the css display property. Make sure you have display: inline or display: inline-block or display: initial.

anna-tsukanova commented 4 years ago

@djizco I'm not using any custom styles for this story. The page contains only the styles of the storybook itself. An interesting note is that when the components were written using knobs, everything worked correctly, but with controls for some reason there is such a problem with spacing (

issuehunt-oss[bot] commented 3 years ago

@kylemh has funded $20.00 to this issue.


tklives commented 3 years ago

Hey y'all!

I am also still very much interested in any potential fixes or workarounds for this issue. Any additional info to share on this yet? :)

Thanks! TK

ndelangen commented 3 years ago

We should now be lazy-loading the syntaxhighlighter.. Maybe we can restart the effort to employ prettier for formatting?

frassinier commented 3 years ago

Here is a workaround for now, by editing your preview.js

import prettier from 'prettier/standalone';
import prettierBabel from 'prettier/parser-babel';

export const parameters = {
  docs: {
    transformSource: input =>
      prettier.format(input, {
        parser: 'babel',
        plugins: [prettierBabel],
      }),
  },
};

Hope it can help!

01taylop commented 3 years ago

@frassinier

Whilst this solution did work for me, running Storybook took a lot longer and I see this getting logged for a number of different files:

Note: The code generator has deoptimised the styling of /PATH/node_modules/prettier/standalone.js as it exceeds the max of 500KB.

It seems the issue occurs as soon as I add the prettier/standalone import.

mpalpha commented 3 years ago

@01taylop My workaround may work for you.

wisteria-hill-technologies commented 2 years ago

Hi, I tried the workaround by @frassinier. But, it added semicolon at the end of the code block and also it did not parse a component with a prop which passes a component, and doc breaks.

Instead, I did below using pretty library:

import pretty from 'pretty';
...
export const parameters = {
  docs: {
     transformSource: input => pretty(input)
  }
  ...
}

For your information. This worked for me.

ndelangen commented 1 year ago

This is fixed in 7.0 beta

MrErHu commented 1 year ago

Ridiculous, this problem still has not been solved in 2023!

ndelangen commented 1 year ago

Would you be so kind to explain to us how you concluded this is in fact not yet fixed in the current latest version?

We can re-open this if you supply a reproduction repo.

PauliCZ44 commented 1 year ago

Here is a workaround for now, by editing your preview.js

import prettier from 'prettier/standalone';
import prettierBabel from 'prettier/parser-babel';

export const parameters = {
  docs: {
    transformSource: input =>
      prettier.format(input, {
        parser: 'babel',
        plugins: [prettierBabel],
      }),
  },
};

Hope it can help!

This work like a charm in SB 7.5

lwkchan commented 11 months ago

It seems like the solution which uses prettier/standalone and prettier/parser-babel above do not work if you use prettier v3 or later. This is because from v3, prettier's format function is asychrnous returning a promise (see release note). However, Storybook's transformSource is expected to be a function which returns a string.

Using 'prettier-synchronized' didn't work for me either, since it looks like it needs to be used in a node environment

JamesIves commented 7 months ago

It seems like the solution which uses prettier/standalone and prettier/parser-babel above do not work if you use prettier v3 or later. This is because from v3, prettier's format function is asychrnous returning a promise (see release note). However, Storybook's transformSource is expected to be a function which returns a string.

Using 'prettier-synchronized' didn't work for me either, since it looks like it needs to be used in a node environment

Also running into this. Curious if there's any workarounds.

karlschwaier commented 4 months ago

I had the same issue and found a workaround by installing Prettier v2 with npm aliasing.

"prettier": "3.3.2",
"prettier-v2": "npm:prettier@2.8.8",

preview.ts:

import prettier from 'prettier-v2';
import HTMLParser from 'prettier-v2/parser-html';
caseywatts commented 1 month ago

I got this working! Prettier parsing of the code preview. Thanks to a lot of clues in this thread, plus this thread

package.json

  "dependencies": {
    "prettier": "^3.3.3",
    "prettier-v2": "npm:prettier@2.8.8"
  },

preview.ts

import type { Preview } from "@storybook/web-components";
import prettier from "prettier-v2";
import HTMLParser from "prettier-v2/parser-html";

const preview: Preview = {
    docs: {
      source: {
        transform: (input) =>
          prettier.format(input, {
            parser: "html",
            plugins: [HTMLParser],
          }),
      },
    },
  },
};

export default preview;

('prettier-synchronized' didn't work for me either)