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

Pull renderer+builder from framework's package.json + a known list #19717

Merged
merged 3 commits into from
Nov 3, 2022

Conversation

tmeasday
Copy link
Member

@tmeasday tmeasday commented Nov 2, 2022

Issue: Framework concept was out of date here.

What I did

  • Pull framework from main:framework.
  • Then search that package's package.json for known dependencies to work out renderer + builder.

This was an approach we thought was a good compromise between reliability and correctness.

How to test

See tests

STORYBOOK_TELEMETRY_DEBUG=1 yarn storybook

@tmeasday tmeasday added feature request telemetry maintenance User-facing maintenance tasks and removed feature request labels Nov 2, 2022
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.

@tmeasday Changes look good to me. Do we want to handle the case where users specify builder or renderer manually in their main.js?

@tmeasday
Copy link
Member Author

tmeasday commented Nov 3, 2022

Is that possible in the new architecture? What's the API (core.builder + core.renderer? Or something else?)?

Copy link
Member

shilman commented Nov 3, 2022

Yes, I think they will override framework settings. Though I haven't verified. Maybe we can tackle that later if we have time

@tmeasday tmeasday merged commit f8f22ac into next Nov 3, 2022
@tmeasday tmeasday deleted the tom/sb-726-telemetry-fix-rendererframework branch November 3, 2022 03:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance User-facing maintenance tasks telemetry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants