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

migrate withStorybook to typescript #633

Merged
merged 6 commits into from
Oct 27, 2024

Conversation

tlow92
Copy link
Contributor

@tlow92 tlow92 commented Oct 26, 2024

Proposal to fix #630

Note

Instead of manually defining the d.ts file I converted the current implementation to typescript and let tsup emit the .js and .d.ts file. This should hopefully be more future-proof.

In general it works, but I had to add a custom tsup cli cmd to the package.json (reason is explained below).

  • Currently withStorybook is imported like this require('@storybook/react-native/metro/withStorybook');
    I think ideally withStorybook would live in src like all other files and be transpiled to dist, but that would be a breaking change.
  • I tried to emit the files to dist and put barrel files in metro folder so that it is not a breaking change, but that created problems with tsup because apparently it can not handle barrel .d.ts files and always emits an empty .d.ts.
  • The other approach is what I did now:
    Using a separate tsup cmd to specify a different outDir (metro folder)

@tlow92 tlow92 requested a review from dannyhw as a code owner October 26, 2024 20:10
@dannyhw
Copy link
Member

dannyhw commented Oct 27, 2024

thanks for getting this underway! I think we got a good solution here. I'm going to merge and start working toward a 8.4 pre-release with multiple changes in it

@dannyhw dannyhw merged commit 37d653f into storybookjs:next Oct 27, 2024
1 check passed
@trajano
Copy link
Contributor

trajano commented Nov 8, 2024

I looked at the code the hack I did with export = withStorybook is not needed if you converted to typescript instead just export function withStorybook in an earlier line.

The hack was needed because of the d.ts nature which no longer applies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

incorrect typing for withStorybook
3 participants