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

[Bug]: Storybook 7 beta 40 crashes if preview storySort is a reference to an array #20883

Closed
kroeder opened this issue Feb 2, 2023 · 6 comments · Fixed by #20930
Closed

[Bug]: Storybook 7 beta 40 crashes if preview storySort is a reference to an array #20883

kroeder opened this issue Feb 2, 2023 · 6 comments · Fixed by #20930

Comments

@kroeder
Copy link
Member

kroeder commented Feb 2, 2023

Describe the bug

In your preview.ts / js, if you previously sort stories like this

const myOrder = ['Foo', 'Bar'];
export const parameters = {
    options: {
        storySort: {
            method: 'alphabetical',
            order: myOrder,
            locales: 'en-US',
        }
    }
};

Storybook crashes with a runtime error

info => Using angular project with "tsConfig:.storybook/tsconfig.json"
ERR! Error: Unknown node type [object Object]
ERR!     at parseValue (/Users/kroeder/dev/Work/cloud-ui/node_modules/@storybook/csf-tools/dist/index.js:74:11123)
ERR!     at /Users/kroeder/dev/Work/cloud-ui/node_modules/@storybook/csf-tools/dist/index.js:74:11047
ERR!     at Array.reduce (<anonymous>)
ERR!     at parseValue (/Users/kroeder/dev/Work/cloud-ui/node_modules/@storybook/csf-tools/dist/index.js:74:10989)
ERR!     at getStorySortParameter (/Users/kroeder/dev/Work/cloud-ui/node_modules/@storybook/csf-tools/dist/index.js:96:130)
ERR!     at StoryIndexGenerator.getStorySortParameter (/Users/kroeder/dev/Work/cloud-ui/node_modules/@storybook/core-server/dist/index.js:10:17587)
ERR!     at async StoryIndexGenerator.sortStories (/Users/kroeder/dev/Work/cloud-ui/node_modules/@storybook/core-server/dist/index.js:10:15509)
ERR!     at async StoryIndexGenerator.getIndex (/Users/kroeder/dev/Work/cloud-ui/node_modules/@storybook/core-server/dist/index.js:10:15846)
ERR!     at async extractStoriesJson (/Users/kroeder/dev/Work/cloud-ui/node_modules/@storybook/core-server/dist/index.js:10:4396)
ERR!     at async Promise.all (index 4)
ERR!  Error: Unknown node type [object Object]
ERR!     at parseValue (/Users/kroeder/dev/Work/cloud-ui/node_modules/@storybook/csf-tools/dist/index.js:74:11123)
ERR!     at /Users/kroeder/dev/Work/cloud-ui/node_modules/@storybook/csf-tools/dist/index.js:74:11047
ERR!     at Array.reduce (<anonymous>)
ERR!     at parseValue (/Users/kroeder/dev/Work/cloud-ui/node_modules/@storybook/csf-tools/dist/index.js:74:10989)
ERR!     at getStorySortParameter (/Users/kroeder/dev/Work/cloud-ui/node_modules/@storybook/csf-tools/dist/index.js:96:130)
ERR!     at StoryIndexGenerator.getStorySortParameter (/Users/kroeder/dev/Work/cloud-ui/node_modules/@storybook/core-server/dist/index.js:10:17587)
ERR!     at async StoryIndexGenerator.sortStories (/Users/kroeder/dev/Work/cloud-ui/node_modules/@storybook/core-server/dist/index.js:10:15509)
ERR!     at async StoryIndexGenerator.getIndex (/Users/kroeder/dev/Work/cloud-ui/node_modules/@storybook/core-server/dist/index.js:10:15846)
ERR!     at async extractStoriesJson (/Users/kroeder/dev/Work/cloud-ui/node_modules/@storybook/core-server/dist/index.js:10:4396)
ERR!     at async Promise.all (index 4)
 >  NX   Broken build, fix the error above.
   You may need to refresh the browser.

Workaround

Use an inline-array instead.

export const parameters = {
    options: {
        storySort: {
            method: 'alphabetical',
            order: ['Foo', 'Bar'],
            locales: 'en-US',
        }
    }
};

To Reproduce

No response

System

System:
    OS: macOS 13.0.1
    CPU: (10) arm64 Apple M1 Pro
  Binaries:
    Node: 16.17.0 - ~/.nvm/versions/node/v16.17.0/bin/node
    Yarn: 1.22.19 - ~/.yarn/bin/yarn
    npm: 8.15.0 - ~/.nvm/versions/node/v16.17.0/bin/npm
  Browsers:
    Chrome: 109.0.5414.119
    Safari: 16.1
  npmPackages:
    @storybook/addon-a11y: ^7.0.0-beta.40 => 7.0.0-beta.40 
    @storybook/addon-essentials: ^7.0.0-beta.40 => 7.0.0-beta.40 
    @storybook/addon-interactions: ^7.0.0-beta.40 => 7.0.0-beta.40 
    @storybook/addons: ^7.0.0-beta.40 => 7.0.0-beta.40 
    @storybook/angular: ^7.0.0-beta.40 => 7.0.0-beta.40 
    @storybook/jest: 0.0.11-next.0 => 0.0.11-next.0 
    @storybook/testing-library: 0.0.14-next.1 => 0.0.14-next.1 
    @storybook/theming: ^7.0.0-beta.40 => 7.0.0-beta.40

Additional context

No response

@zhyd1997
Copy link
Contributor

zhyd1997 commented Feb 3, 2023

Yes, the source code is here:

Unexpected '${unexpectedVar}'. Parameter 'options.storySort' should be defined inline e.g.:

@shilman
Copy link
Member

shilman commented Feb 4, 2023

@kroeder the code linked to by @zhyd1997 explains the problem with your configuration. I'm wondering why the error in your bug report doesn't show that error.

@yannbf can we add this to the SB eslint rules for preview.js?

@kroeder
Copy link
Member Author

kroeder commented Feb 4, 2023

It does

ERR! at async extractStoriesJson

Second last error message.
The error itself makes sense as well

@ndelangen and @valentinpalkovic found out, that the implementation of storySort does not cover references to an array and only a direct array assignment which I would consider a bug that is solvable (you removed the bug label)

I can live with the workaround, though :)

Copy link
Member

shilman commented Feb 4, 2023

@kroeder I don't see the string "Parameter 'options.storySort' should be defined inline" in your report.

Regarding whether this is a bug, it's the intended behavior. We've defined the limitations of the code and give an error explaining what to do to fix it.

Yes we could chase down the variable reference but where do you stop? What if it's a reference to a variable that references a variable? What if it's a variable that references an import from another file?

@zhyd1997
Copy link
Contributor

zhyd1997 commented Feb 4, 2023

// preview.js
const myOrder = ['Foo', 'Bar'];
export const parameters = {
    options: {
        storySort: {
            method: 'alphabetical',
            order: myOrder,
            locales: 'en-US',
        }
    }
};

t.isObjectExpression(storySort) but t.isIdentifier(p.value):

Screenshot 2023-02-04 at 17 53 43

which is not handled by parseValue fn.

if (t.isObjectExpression(expr)) {
return expr.properties.reduce((acc, p: t.ObjectProperty) => {
if (t.isIdentifier(p.key)) {
acc[p.key.name] = parseValue(p.value as t.Expression);
}
return acc;
}, {} as any);
}

that's why throws the error unknown node type:

throw new Error(`Unknown node type ${expr}`);

This change would be better:

change ${expr} to ${expr.type}.

Screenshot 2023-02-04 at 17 50 27

@shilman
Copy link
Member

shilman commented Feb 7, 2023

Great Caesar's ghost!! I just released https://github.com/storybookjs/storybook/releases/tag/v7.0.0-beta.44 containing PR #20930 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb@next upgrade --prerelease

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants