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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Composition: Filter out disabled refs in getAutoRefs #12863

Merged
merged 7 commits into from Dec 1, 2020

Conversation

g1eny0ung
Copy link
Contributor

@g1eny0ung g1eny0ung commented Oct 22, 2020

Issue:

If some packages don鈥榯 export package.json, start storybook will warn:

image

What I did

I think to solve this problem we can define these packages in the refs and set it as disabled in .storybook/main.js, like:

refs: {
  rollup: { disable: true },
  '@rollup/plugin-node-resolve': { disable: true },
}

Then filter out the disabled refs in getAutoRefs func. Also, the changed code will reduce the extra disabled refs check than the previous code.

I'm not sure if this is correct or reasonable, please help me point out if there is an error, thx. 馃槂

How to test

  • Is this testable with Jest or Chromatic screenshots?
  • Does this need a new example in the kitchen sink apps?
  • Does this need an update to the documentation?

If your answer is yes to any of these, please make sure to include it in your PR.

@g1eny0ung
Copy link
Contributor Author

Hi @shilman, is there any progress on this PR?

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

LGTM!

@ndelangen
Copy link
Member

it's possible for an npm package to not contain a package.json?!?!

@g1eny0ung
Copy link
Contributor Author

it's possible for an npm package to not contain a package.json?!?!

@ndelangen They actually have the package.json. The reason I guess is both rollup and @rollup/plugin-node-resolve defined the exports field to limit the scope of resolve.

@g1eny0ung
Copy link
Contributor Author

@shilman @ndelangen I noticed the latest changes use typescript and I have already resolved the conflicts. Please check again.

@shilman shilman changed the title Fix: filter out the disabled refs in getAutoRefs Composition: Filter out disabled refs in getAutoRefs Dec 1, 2020
@shilman shilman merged commit 17826e3 into storybookjs:next Dec 1, 2020
@g1eny0ung g1eny0ung deleted the refactor/getAutoRefs branch December 1, 2020 10:06
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.

None yet

3 participants