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 project source to TypeScript #346

Merged
merged 3 commits into from
Mar 29, 2023
Merged

Conversation

carloskelly13
Copy link
Contributor

@carloskelly13 carloskelly13 commented Mar 29, 2023

  • Migrated project source and Storybook Stories over to TypeScript

How to verify:

  1. Build library via yarn build
  2. Run Storybook

@vercel
Copy link

vercel bot commented Mar 29, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
react-live ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Mar 29, 2023 at 10:39PM (UTC)

Copy link
Contributor

@gksander gksander left a comment

Choose a reason for hiding this comment

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

Couple quick comments. Also, I'm noticing the following when running storybook (Function No Inline Example). I very well might be running something wrong though.

image

CHANGELOG.md Outdated Show resolved Hide resolved
Comment on lines 5 to +8
import { terser } from "rollup-plugin-terser";
import filesize from "rollup-plugin-filesize";
import { visualizer } from "rollup-plugin-visualizer";
import typescript from "@rollup/plugin-typescript";
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to change this now, but in the future we might look into swapping rollup out for Vite (which uses rollup under the hood), just because the plugin/config is a bit less verbose. Maybe something for "future us".

@gksander
Copy link
Contributor

LOL okay, makes total sense. I wasn't paying close enough attention.

Copy link
Contributor

@gksander gksander left a comment

Choose a reason for hiding this comment

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

Lgtm

@carloskelly13
Copy link
Contributor Author

@gksander Actually you helped me uncover a bug, it wasn't passing the correct value for noInline in the story—it just looked correct because of default params and how it saves transient stage in local storage. Thank you for calling that out. Pushed a fix for that.

@carloskelly13 carloskelly13 merged commit a1cd579 into master Mar 29, 2023
@carloskelly13 carloskelly13 deleted the task/convert-source-ts branch March 29, 2023 22:51
@github-actions github-actions bot mentioned this pull request Mar 29, 2023
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.

2 participants