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

refactor: Composition APIへ移行 #8121

Merged
merged 26 commits into from
Jan 10, 2022
Merged

refactor: Composition APIへ移行 #8121

merged 26 commits into from
Jan 10, 2022

Conversation

tamaina
Copy link
Contributor

@tamaina tamaina commented Jan 6, 2022

#7681

What

Composition APIに書き換える作業用。1コンポーネントにつき100円もらえると聞いて

@syuilo
Copy link
Member

syuilo commented Jan 6, 2022

setup sugar使うとより簡潔に書けそう
https://v3.vuejs.org/api/sfc-script-setup.html

@syuilo
Copy link
Member

syuilo commented Jan 6, 2022

さらにref sugar使うとより簡潔に書けるけど現時点では個人的には逆に分かりにくくて好みじゃないかも(あとまだRFCだから仕様が変わる可能性がある)
vuejs/rfcs#369

@tamaina
Copy link
Contributor Author

tamaina commented Jan 6, 2022

↑こんな感じかしら

@syuilo
Copy link
Member

syuilo commented Jan 6, 2022

↑こんな感じかしら

👍

@tamaina
Copy link
Contributor Author

tamaina commented Jan 7, 2022

analog-clock、テーマが変わっても色がそのまんまな仕様なんだな

@acid-chicken
Copy link
Member

acid-chicken commented Jan 7, 2022

analog-clock、テーマが変わっても色がそのまんまな仕様なんだな

仕様というか仕様バグ感ある(getComputedStyle でスタイルの変更を検知する術がないので)
やるとしたら別途スタイル変更を通知できるようにするか、あるいは MutationObserver でスタイルの変更を見張るか(サブツリーまで見ると重いと思うのでトップレベルだけ見るようなルーズな実装が落とし所かと思ったけど親ノード全部ぶちこむだけだったらそんな大規模にはならなさそう)
computed 内で直接 getPropertyValue を挟まなければ良いだけだった

@syuilo
Copy link
Member

syuilo commented Jan 7, 2022

テーマ切り替えしたときにグローバルなイベント発行してたはずだからそれListenして再レンダリングするようにする予定だった

@tamaina
Copy link
Contributor Author

tamaina commented Jan 8, 2022

captcha設定画面、hcaptcha開いた後recaptcha開くとなぜかhcaptchaのプレビューが表示される

@tamaina
Copy link
Contributor Author

tamaina commented Jan 8, 2022

(compositionじゃなくても)

recaptchaのあとにhcaptchaのパターンは正常なのでなんかよくわからん

@acid-chicken
Copy link
Member

captcha設定画面、hcaptcha開いた後recaptcha開くとなぜかhcaptchaのプレビューが表示される

hcaptcha は grecaptcha を上書きする挙動だったはず

@acid-chicken
Copy link
Member

#6303 (comment)

@syuilo
Copy link
Member

syuilo commented Jan 8, 2022

packages/client/src/components/autocomplete.vue みたいな<script setup>移行前にトップレベルでデータの準備していたようなケースだと、<script setup>移行後にコンポーネントが作成されるたびにデータの準備が行われることになってパフォーマンスの問題が発生しそう

<script setup>とは別に<script>を用意することで回避できるっぽいけどやったことないから詳細は不明

@tamaina
Copy link
Contributor Author

tamaina commented Jan 8, 2022

<script>で宣言した変数を参照できない(?)ので素直にdefineComponentしたほうがよさそう

@syuilo
Copy link
Member

syuilo commented Jan 9, 2022

@tamaina pagination.vue と paging.ts は私がやる

@tamaina
Copy link
Contributor Author

tamaina commented Jan 9, 2022

abc順にやってるから私が手を付けるとしてもまだ先だと思う

@tamaina
Copy link
Contributor Author

tamaina commented Jan 9, 2022

<script>で宣言した変数を参照できない(?)ので

export defaultしなきゃいけないのを見逃していた

@tamaina
Copy link
Contributor Author

tamaina commented Jan 10, 2022

これってマージされるんやろか
(見つけたバグの修正もしたかったりする(compositon-workに対してプルリク作ればいいんだろうけどこれ以上プルリクを抱えるのがきつい)

@syuilo
Copy link
Member

syuilo commented Jan 10, 2022

マージするか

@syuilo syuilo self-requested a review January 10, 2022 14:40
@syuilo syuilo merged commit 8855a5f into develop Jan 10, 2022
@syuilo syuilo deleted the composition-work branch January 10, 2022 15:05
@syuilo
Copy link
Member

syuilo commented Jan 10, 2022

🙏🙏🙏

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

3 participants