Addons seems to not respect import order

jonhuteau commented 4 years ago

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

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

matt-shine commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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

ndelangen commented 4 years ago

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

ndelangen commented 4 years ago

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 commented 4 years ago

Here's the current code:

https://github.com/storybookjs/storybook/blob/65ab7e238773229f02f13e4674ff1fa7431d7719/lib/core/src/server/presets.js#L151-L164

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

ndelangen commented 4 years ago

YES! a solution at last, I think.

shilman commented 4 years ago

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 commented 4 years ago

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

shilman commented 4 years ago

@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 commented 4 years ago

@shilman got it. thanks for the answer!

storybookjs / storybook

Addons seems to not respect import order #9819