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

Add: types.ts #725

Merged
merged 4 commits into from
Mar 17, 2022
Merged

Add: types.ts #725

merged 4 commits into from
Mar 17, 2022

Conversation

takurinton
Copy link
Contributor

ref #716 (comment)

tsconfig の skipLibCheck の対応として、DeepPartial を export する。
現状、共通の型を置いておく場所がないので types/types.ts を追加した。

@netlify
Copy link

netlify bot commented Feb 24, 2022

✔️ Deploy Preview for ingred-ui ready!

🔨 Explore the source changes: 93cfe69

🔍 Inspect the deploy log: https://app.netlify.com/sites/ingred-ui/deploys/623298ec47191d0008a8cf15

😎 Browse the preview: https://deploy-preview-725--ingred-ui.netlify.app

@takurinton
Copy link
Contributor Author

types/index.ts でも良かったかもしれない。

@@ -0,0 +1,4 @@
// MEMO: from Redux
export type DeepPartial<T> = {
Copy link
Contributor

Choose a reason for hiding this comment

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

DeepPartial.d.tsみたいなファイル名の方が良いかな?(とにかく型はこのファイルに突っ込んでください!みたいなやり方を見たことがない)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

MUI は index.d.ts にしてるっぽいですね。
MUI に合わせます。
https://github.com/mui/material-ui/blob/master/packages/mui-types/index.d.ts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(別パッケージとして分離はしてるっぽいですが)

@youchann
Copy link
Contributor

@takurinton
あーこの型、ライブラリの利用者側でも使える方が良かったりするかな?:thinking:
現状だとimportできないよねぇ

Copy link
Contributor

@youchann youchann left a comment

Choose a reason for hiding this comment

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

#725 (comment)
↑気づくの遅くて恐縮だけどみていただけると

@takurinton
Copy link
Contributor Author

@youchann
現状だと import できないです。
一般的にどう使われていたりとか、どう定義されているかをちょっと調べてみます!!

補足ですが、個人的にはユーザー側で定義すればいいと思っていましたが、どうせ定義してるなら使えた方がいいのかなとかも思ったりしました。
これを機に export してしまうのもありかもしれないですね。

@youchann
Copy link
Contributor

内部で使っているreact-selectなどなどは、型もexportしてる(ingred-uiもpropsは全部exportしてる)から、まぁしてもよさそう

@takurinton
Copy link
Contributor Author

d.ts が読めない。

@takurinton
Copy link
Contributor Author

MUI を参考にしようと思ったけど、あれはあれ自体がパッケージだからできてたけど、これをバンドルしようとすると rollup-plugin-dts あたりでうまく分割しないといけないっぽいので一旦スルー。

@takurinton takurinton merged commit cf7c0c0 into master Mar 17, 2022
@takurinton takurinton deleted the add/types-dir branch March 17, 2022 02:35
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.

None yet

2 participants