storybook: `import * as x from '...'` does not work as intended anymore (due to `__namedExportsOrder` being exposed)

Describe the bug

The following code should return a Module object x. x’s keys, values etc. should only be that of the named exports from given file. However, due to a recent change, __namedExportsOrder is now also an enumerable property of that object, and messes with any code that assumes default Module behavior.

The feature that brought this about: https://github.com/storybookjs/storybook/issues/9136
The commit that probably broke it: https://github.com/storybookjs/storybook/commit/8d826f99cf651ef00624a1c190160ece772065b5
Relevant webpack loader code here

To Reproduce

Given two files:

// x.js
export const a = 1;
export const b = 2;

// test.js
import * as x from './x';

console.log(x.__namedExportsOrder);                 // that new prop exists (array of strings, containing the named export names, in order)
const n = Object.keys(x).length;
console.assert(n === 2);   // any attempt to iterate over `x` in anyway is buggy because n is `3`, but should be `2`

System Please paste the results of npx sb@next info here.

  System:
    OS: Windows 10 10.0.19044
    CPU: (8) x64 Intel(R) Core(TM) i7-10510U CPU @ 1.80GHz
  Binaries:
    Node: 16.13.1 - ~\AppData\Local\Volta\tools\image\node\16.13.1\node.EXE
    Yarn: 1.22.17 - ~\AppData\Local\Volta\tools\image\yarn\1.22.17\bin\yarn.CMD
    npm: 8.1.2 - ~\AppData\Local\Volta\tools\image\node\16.13.1\npm.CMD
  Browsers:
    Edge: Spartan (44.19041.1266.0), Chromium (98.0.1108.56)
  npmPackages:
    @storybook/addon-actions: ^6.5.0-alpha.41 => 6.5.0-alpha.42       
    @storybook/addon-essentials: ^6.5.0-alpha.41 => 6.5.0-alpha.42    
    @storybook/addon-links: ^6.5.0-alpha.41 => 6.5.0-alpha.42
    @storybook/addon-postcss: ^2.0.0 => 2.0.0
    @storybook/builder-webpack5: ^6.5.0-alpha.41 => 6.5.0-alpha.42    
    @storybook/manager-webpack5: ^6.5.0-alpha.41 => 6.5.0-alpha.42    
    @storybook/react: ^6.5.0-alpha.41 => 6.5.0-alpha.42

Possible Solution

Either make that property non-enumerable?
Or don’t apply it to any modules that are not loaded by the story loader?

About this issue

Original URL
State: closed
Created 2 years ago
Comments: 21 (11 by maintainers)

Most upvoted comments

It’s the export * part.

It’s valid JS but we don’t support that in CSF currently. To explain why, let me dive into some details about Storybook’s default 7.0 behavior.

One of the biggest requests for Storybook is to improve its boot-up behavior. Storybook’s boot-up time is largely a function of webpack, the default builder that we use. The only way we can make things dramatically faster is if we build less. So in 6.4 we introduced the on-demand architecture and followed it up in 6.5 with lazy compilation.

How it works is that Storybook does a static analysis of your CSF files. That means we parse the contents of the file and analyze its default export and the named exports in the file. We use this to build an index of all the stories in your storybook. Critically, we don’t execute the code in your story files.

This means that some valid JS is not actually valid CSF as far as Storybook is concerned. For example, if you try to use a template literal string in the default.title property, we will throw an error.

As for the export * pattern, figuring out the named exports would require that we load in the file that is being re-exported to figure out the named exports. This is not something we currently do. We might add it as a feature in the future, but for now it will break, so I’m just pointing that out.

The following workaround would probably work, since we wouldn’t need to load ./ConfirmBasic to see what its contents are. Yes, it’s more work:

export { ConfirmBasic1, ConfirmBasic2 } from './ConfirmBasic';

shilman on Apr 14, 2022

Ok, I see what’s going on. There’s actually a FIXME in the code, referenced above, which is that this logic should only be applied to *.stories.* files. I guess the behavior you’re seeing is a consequence of that FIXME not being implemented, so I can try to take a look at that this week.

That said, the pattern you’re using in #17588 isn’t officially supported, so I’m not sure whether it’s a good long-term approach, i.e. it will break in 7.0’s default configuration (but will be supported in a legacy mode)

shilman on Feb 27, 2022

@jdelStrother fix coming in the next 30min

shilman on May 20, 2022

Jeepers creepers!! I released https://github.com/storybookjs/storybook/releases/tag/v6.5.4-alpha.0 containing PR #18284 that references this issue. Upgrade today to the @prerelease NPM tag to try it out!
npx sb upgrade --prerelease
Closing this issue. Please re-open if you think there’s still more to do.

@shilman thanks, looks good

jdelStrother on May 20, 2022

Jeepers creepers!! I released https://github.com/storybookjs/storybook/releases/tag/v6.5.4-alpha.0 containing PR #18284 that references this issue. Upgrade today to the @prerelease NPM tag to try it out!

npx sb upgrade --prerelease

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

shilman on May 20, 2022

This has broken our storybook, where stories use a mobx-state-tree store and the store uses this import to load a set of generic models. import * as markTypes from '@plugins/drawingTools/models/marks/index.js'

I can hack our code to remove the __namedExportsOrder key that has been added to markTypes but the storybook shouldn’t be modifying that import really.

eatyourgreens on May 19, 2022

Thanks for the info!

Would probably be a good idea to explicitly document such deviations from expectation?

I think, in order to reduce boiler-plate while not re-exporting, I’ll remove index.js and replace it with some sort of _stories.common.js that has the export default and will be re-exported by all story files. A lot less work and a lot less error prone than specifically naming every re-export.

Domiii on Apr 14, 2022