-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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 monorepo to strict TS #22176
Comments
Hi, Thanks in advance for your reply. |
@kuriacka Sounds great, one PR or multiple are both fine! Let me now if you start work on any, then I will put your in the assign list so that me (and others) won't do the same work! |
@kasperpeulen Thanks for info, I can start with aforementioned postinstall and router, then I can pick another packages. |
@kasperpeulen Here is the PR for postinstall and router. Thanks! |
Hey @kasperpeulen! I'd like to help too, but I'm not very familiar yet with all the Storybook packages. Which one do you think can be a good one to start? Thanks! |
Hi @kasperpeulen! I'd like to help too! |
@mariasimo @suiux I think the packages in the frameworks directory are probably the easiest! |
@kasperpeulen thanks! I can take |
Sounds great @mariasimo, I assigned them to you :) |
w00t!! I just released https://github.com/storybookjs/storybook/releases/tag/v7.1.0-alpha.9 containing PR #22181 that references this issue. Upgrade today to the
Closing this issue. Please re-open if you think there's still more to do. |
Wait, aren't there a lot more packages that aren't migrated to strict mode yet? I wanted to contribute to @storybook/cli |
@0916dhkim Reopened! |
Apologies! An automation went rogue 😅 Also, for anyone contributing to this or interested in taking part: we've just set up a new Discord working group for the project! Share updates on how you're getting, ask any questions, or simply chat with contributors 🎀 https://discord.com/channels/486522875931656193/1100689736022106173 |
I'll take |
I'm looking very intently at |
PR for the storybook/components #22569 |
May I pick up the @storybook/addons if it's still free? |
Is @storybook/blocks still open or is it taken up by anybody else? If not can i work on it? |
@storybook/addons-mdx-gfm is finished. I just had to switch @storybook/addons-outline looks like it already has strict TS. Let me know if that is not the case. May I take @storybook/angular if no one took it, please? |
ZOMG!! I just released https://github.com/storybookjs/storybook/releases/tag/v7.1.0-alpha.27 containing PR #22399 that references this issue. Upgrade today to the
|
I'm looking into @storybook/manager-api (PR: #22973) and @storybook/manager as I see those are still free |
Could I take @storybook/codemod? |
I've started working on @storybook/core-server since it appears to be free. |
I would like to help with @storybook/codemod |
i want to help in @storybook/manager |
Ok looks like I didn't finish @storybook/codemod in time, I'll pick up @storybook/core-server |
Hi @cupton15 - I already have a PR open for core-server. |
Hello @kasperpeulen, I'm trying to migrate |
@LuisChiej I think we can actually skip external docs. I think this is a package that we likely want to remove in the feature, maybe @shilman has more context about that. |
@kasperpeulen hey I would like to help. Following @storybook/manager-api is still free. can you assign it to me ? |
@subhajit20 Done! |
Hey, I'd like to help out with this work. Of the remaining unassigned packages it looks like codemod and scripts are the ones left to do:
I can look at either, @kasperpeulen any preference? |
What
We want to step our TS game in the monorepo and enable strict typescript in all packages!
Why
Having TS track for you if a variable might be null or not, enables us to code with much more confidence,
and also gives us quick in editor feedback, when you make assumptions that are not actually true!
How
Feel free to contribute to any of packages in the list below!
After you cloned the repo, you can best run this command:
yarn task --task check --no-link --start-from=install
This will install all dependencies, compile it (including dts files, that is what no-link does), and then check typescript errors in all packages.
To enable strict mode you will need:
tsconfig.json
"strict": false
->"strict": true
yarn check
in the directory of the package.We would like to change as little as possible of the actual runtime behavior in this migration.
However, we also don't want to simply silence the compiler in every error with
!
,as
orts-ignore
to get this migration in.As a rule of thumb, if the logic is easy enough, prefer improving the code (e.g. add a null check, throw when not null, possibly use https://github.com/alexreardon/tiny-invariant) over silencing the compiler.
If the change needed to do the right thing, is too risky, and not in your expertise, it is okay to silence the compiler.
It is not ideal, but we still gain the benefit that new code written will have extra typesafety.
So if you'd like to help converting any of these♥️ ! Please mention here, which ones, you can take on, and open a PR migrating a package.
If you need any help, please reach out to storybook maintainers on the contributing channel on the Storybook discord.
The text was updated successfully, but these errors were encountered: