Skip to content

Termタグのtypeを省略できるように#561

Merged
aster-void merged 17 commits into
masterfrom
auto-type-term-tags
Nov 8, 2023
Merged

Termタグのtypeを省略できるように#561
aster-void merged 17 commits into
masterfrom
auto-type-term-tags

Conversation

@aster-void
Copy link
Copy Markdown
Contributor

一部 (CSSプロパティなどの文字から判断できないもの、「truthyとfalsy」など使われ方の不明なものなど) 対応不可能なのでその場合は明示的に書いてください。

"Iterator value undefined is not an iterable object"
buildはなぜか通らないけど自分のせいじゃないです
@aster-void
Copy link
Copy Markdown
Contributor Author

今は動くはずです

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Nov 7, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 6925790
Status: ✅  Deploy successful!
Preview URL: https://e8317931.utcode-learn.pages.dev
Branch Preview URL: https://auto-type-term-tags.utcode-learn.pages.dev

View logs

@aster-void
Copy link
Copy Markdown
Contributor Author

prettierかけ忘れましたw

@aster-void aster-void requested a review from chvmvd November 7, 2023 13:17
Comment thread src/components/Term/short-definitions.js Outdated
Comment thread src/components/Term/auto-type.js Outdated
Comment thread src/components/Term/index.jsx Outdated
Comment thread src/components/Term/index.jsx Outdated
Comment thread src/components/Term/index.jsx Outdated
Comment thread src/components/Term/type-map.js Outdated
Comment thread src/components/Term/type-map.js Outdated
Comment thread src/components/Term/type-map.js Outdated
Comment thread src/components/Term/type-map.js Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

短縮系を作るくらいなら全部短縮系の方がいいと思います:eyes:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

別 PR で短縮形(というかjavascriptなし)とjavascriptありを入れ替える予定です

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

そもそもtypeを指定することはほとんどないので、短縮形にしなくても良くないですか?使うところはjavascriptPropertycssPropertyで短縮できないところとかだから。

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

まあ、確かに...じゃあいいか

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

とりあえず今回は見逃しますが、type-map内だけでも気になるので今度別PRでやります。

Copy link
Copy Markdown
Contributor

@chvmvd chvmvd Nov 8, 2023

Choose a reason for hiding this comment

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

短縮できるところとできないところができてどれを省略するのか決めるの大変だから、そのままで良いんじゃない?

Copy link
Copy Markdown
Contributor Author

@aster-void aster-void Nov 8, 2023

Choose a reason for hiding this comment

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

むしろ今の状態のほうが javascript が入るかどうか考えるのが大変だと思います。
・html 系のタームは html が入ってない
・DOM・イベント・イベントハンドラ は javascriptが入ってない
・ほとんどの言語共通の「関数」には入ってるのに、より狭い「アロー関数」には入ってない。
などバラバラので、パリティすると考えれば

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

たしかに。じゃあ、消しちゃおう。

Comment thread src/components/Term/index.jsx Outdated
Comment thread src/components/Term/index.jsx Outdated
Comment thread src/components/Term/index.jsx Outdated
Comment thread src/components/Term/type-map.js Outdated
Copy link
Copy Markdown
Contributor

@chelproc chelproc left a comment

Choose a reason for hiding this comment

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

よさそうですね!!

Comment thread src/components/Term/type-map.js Outdated
Comment thread src/components/Term/type-map.js Outdated
Comment thread src/components/Term/type-map.js Outdated
@aster-void aster-void merged commit 90c6dd6 into master Nov 8, 2023
@aster-void aster-void deleted the auto-type-term-tags branch November 8, 2023 08:04
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.

3 participants