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

Addons seems to not respect import order #9819

Closed
jonhuteau opened this issue Feb 11, 2020 · 11 comments
Closed

Addons seems to not respect import order #9819

jonhuteau opened this issue Feb 11, 2020 · 11 comments
Assignees
Milestone

Comments

@jonhuteau
Copy link

Describe the bug

Addons seems to not respect import order as stated in the doc

The tab order is created by order in which they appear in the array in the main.js file.
https://storybook.js.org/docs/addons/using-addons/#addons-tab-order

Expected behavior

When I define addons, I want addons to respect this order (knobs, jest, a11y)

module.exports = {
  addons: [
    '@storybook/addon-docs',
    '@storybook/addon-knobs/register',
    '@storybook/addon-viewport/register',
    '@storybook/addon-jest/register',
    '@storybook/addon-a11y',
  ],
  stories: [
    '../packages/**/*.stories.(js|mdx)',
  ],
}

Screenshots

image

System:

Environment Info:

  System:
    OS: Linux 4.15 Ubuntu 18.04.3 LTS (Bionic Beaver)
    CPU: (8) x64 Intel(R) Core(TM) i7-8650U CPU @ 1.90GHz
  Binaries:
    Node: 12.14.1 - /usr/bin/node
    npm: 6.13.4 - /usr/bin/npm
  Browsers:
    Chrome: 79.0.3945.130
    Firefox: 72.0.2
  npmPackages:
    @storybook/addon-a11y: ^6.0.0-alpha.9 => 6.0.0-alpha.9 
    @storybook/addon-docs: ^6.0.0-alpha.9 => 6.0.0-alpha.9 
    @storybook/addon-jest: ^6.0.0-alpha.9 => 6.0.0-alpha.9 
    @storybook/addon-knobs: ^6.0.0-alpha.9 => 6.0.0-alpha.9 
    @storybook/addon-viewport: ^6.0.0-alpha.9 => 6.0.0-alpha.9 
    @storybook/vue: ^6.0.0-alpha.9 => 6.0.0-alpha.9 

Additional context
This is a VueJS storybook

@shilman shilman added this to the 5.3.x milestone Feb 11, 2020
@matt-shine
Copy link

Mildly helpful (I started to investigate this but have run out of time for now):

/lib/core/src/server/preset.js

loadPreset() calls splitAddons(), which in my project (where I see the same issue), results in:

{ managerEntries:
   [ '/Users/mshine/code/storybook/addons/viewport/register.js',
     '/Users/mshine/code/storybook/addons/a11y/register.js',
     '/Users/mshine/code/storybook/addons/links/register.js' ],
  presets:
   [ '/Users/mshine/code/storybook/addons/docs/preset.js',
     '/Users/mshine/code/storybook/addons/knobs/preset.js' ] }

my addons array:

addons: [
    '@storybook/addon-docs',
    '@storybook/addon-knobs',
    '@storybook/addon-viewport',
    '@storybook/addon-a11y',
    '@storybook/addon-links',
  ],

So this at least points to where/why the issue is happening

@shilman
Copy link
Member

shilman commented Feb 14, 2020

@matt-shine Thanks so much for tracking that down. Unfortunately it's not a straightforward fix. I'll see what @ndelangen and I can come up with there.

@ndelangen
Copy link
Member

I see.. Will give this some thought as well.

@ndelangen
Copy link
Member

We should likely extend the sorting I added so the order is correct. (according to original addons array)

@ndelangen
Copy link
Member

So there's 2 methods for adding managerEntries:

  • via a preset (order is respected)
  • via register (all of this type will be appended (internal order respected)

So if you have:

  addons: [
    '@storybook/addon-docs', // preset
    '@storybook/addon-storysource', // preset
    '@storybook/addon-design-assets', // register
    '@storybook/addon-actions', // preset
    '@storybook/addon-links', // preset
    '@storybook/addon-events', // register
    '@storybook/addon-knobs', // preset
    '@storybook/addon-controls', // preset
    '@storybook/addon-cssresources', // register
    '@storybook/addon-backgrounds', // register
    '@storybook/addon-a11y', // preset
    '@storybook/addon-jest', // register
    '@storybook/addon-viewport', // register
    '@storybook/addon-graphql', // register
    '@storybook/addon-toolbars', // preset
    '@storybook/addon-queryparams', // preset
  ],

the resulting load order of addons is:

  addons: [
    '@storybook/addon-docs', // preset
    '@storybook/addon-storysource', // preset
    '@storybook/addon-actions', // preset
    '@storybook/addon-links', // preset
    '@storybook/addon-knobs', // preset
    '@storybook/addon-controls', // preset
    '@storybook/addon-a11y', // preset
    '@storybook/addon-toolbars', // preset
    '@storybook/addon-queryparams', // preset
    '@storybook/addon-design-assets', // register
    '@storybook/addon-events', // register
    '@storybook/addon-cssresources', // register
    '@storybook/addon-backgrounds', // register
    '@storybook/addon-jest', // register
    '@storybook/addon-viewport', // register
    '@storybook/addon-graphql', // register
  ],

@ndelangen
Copy link
Member

Here's the current code:

const subPresets = resolvePresetFunction(presetsInput, presetOptions, storybookOptions);
const subAddons = resolvePresetFunction(addonsInput, presetOptions, storybookOptions);
const { managerEntries, presets } = splitAddons(subAddons);
return [
...loadPresets([...subPresets, ...presets], level + 1, storybookOptions),
{ name: `${name}_additionalManagerEntries`, preset: { managerEntries } },
{
name,
preset: rest,
options: presetOptions,
},
];

We need to ensure the merging of subPresets & preset happen in the order specified in subAddons

@ndelangen
Copy link
Member

YES! a solution at last, I think.

@shilman
Copy link
Member

shilman commented Jun 16, 2020

Jeepers creepers!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.0.0-beta.30 containing PR #11178 that references this issue. Upgrade today to try it out!

You can find this prerelease on the @next NPM tag.

@AleshaOleg
Copy link

Sorry if I'm missed something, but can it be fixed also for version 5.x.x?

@shilman
Copy link
Member

shilman commented Jul 2, 2020

@AleshaOleg unfortunately it's a significant change. 6.0 will be in RC in a few days, and i'd recommend upgrading once that lands

@AleshaOleg
Copy link

@shilman got it. thanks for the answer!

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