Core: Support valid globs #10926
Core: Support valid globs #10926
Conversation
…ntation & migration-guide
# Conflicts: # MIGRATION.md
ndelangen
requested review from
alexandrebodin,
alterx,
igor-dv,
stijnkoopal,
tmeasday and
usulpro
as code owners
|
7.0 or later yes |
| '../src/stories/components/Icon/Icon.mdx', | ||
| ], | ||
| }, | ||
| // DUMB GLOB |
There was a problem hiding this comment.
should this comment stay here? 😂
|
So turns out, valid globs break webpack. I've investigating why, but it's not going well. |
Co-authored-by: Yann Braga <[email protected]>
Co-authored-by: Yann Braga <[email protected]>
Co-authored-by: Yann Braga <[email protected]>
Co-authored-by: Michael Shilman <[email protected]>
# Conflicts: # lib/core/package.json
|
So happy to have a found a solution, and a clean break on how to deal with the invalid globs in a backwards but future friendly way. All we need to do to remove the bad behavior support is delete a few lines of code. |
Co-authored-by: Michael Shilman <[email protected]>
Co-authored-by: Michael Shilman <[email protected]>
| @@ -57,13 +57,18 @@ export async function createPreviewEntry(options) { | |||
| if (stories && stories.length) { | |||
| entries.push(path.resolve(path.join(configDir, `generated-stories-entry.js`))); | |||
|
|
|||
| const files = (await Promise.all(stories.map((g) => glob(g)))).reduce((a, b) => a.concat(b)); | |||
| const files = ( | |||
| await Promise.all(stories.map((g) => glob(path.join(configDir, g)))) | |||
There was a problem hiding this comment.
@ndelangen Is this correct? I don't think it makes sense to join the glob with the configDir. This means that all globs before this pr won't work anymore
There was a problem hiding this comment.
yes this is intentional indeed.
There was a problem hiding this comment.
@ndelangen if this is a breaking change do we need to document it in MIGRATION.md?
There was a problem hiding this comment.
This is def a breaking change that should be documented. But it seems really odd to me that your glob become .storybook/**/*.stories. IDK when I would ever want that. In reality it always must be something like .storybook/../**/*.stories.
Issue: #10772
What I did
The value of
storiesinmain.jsis supposed to be a valid glob. We plan to optimize storybook by moving from a regex evaluated in webpack to a glob evaluated by node.But we made a mistake, when we wrote the original globs in our docs & CLI generators. It's actually an invalid glob, but it still works, because the glob-to-regex lib makes it work.
In the future instead of using a glob-to-regex we'll use glob solely.
I added instructions to migration.md & make the warning clearer on what's really happening.
We'll keep the invalid globs working in storybook for now, but they will break at some point in the future.