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

Addon-docs: Fix MDX IDs from CSF imports #12154

Merged
merged 1 commit into from
Aug 20, 2020

Conversation

shilman
Copy link
Member

@shilman shilman commented Aug 20, 2020

Issue: #12143

This is technically a breaking change but I'd like to treat it as a bugfix since the feature is brand new.

What I did

This is a cheap/hacky way to make sure that the variable is unique.

Consider the following MDX:

import { foo } from './button.stories';

<Story story={foo} />

The compiler was generating the following code (simplified):

import { foo } from './button.stories';

// lots of MDX stuff omitted

export const fooStory = foo;

If I made the export named foo, it would collide with the import name and the generated code would be invalid. However, this modified the Story ID.

This PR modifies the generated code to be:

export const _foo_ = foo;

Which preserves the same ID.

How to test

See attached snapshot & CSF imported MDX example in official-storybook.

@shilman shilman added this to the 6.0.x milestone Aug 20, 2020
Copy link
Member

@tmeasday tmeasday left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I guess. Obviously would be better to just transform the import to

import { foo as fooImport } ...

But I guess that's way more complicated.

Was this a new feature in 6.0? Didn't we have this in 5.3?

@shilman
Copy link
Member Author

shilman commented Aug 20, 2020

Yeah, more complicated ☹️

<Story story={ xxx } /> is new in 6.0

@shilman shilman merged commit 735848f into next Aug 20, 2020
@stof stof deleted the 12143-fix-mdx-ids-from-csf-imports branch May 25, 2022 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants