Skip to content
This repository has been archived by the owner on Jan 13, 2023. It is now read-only.

Fix build config for symlinking app common #278

Merged
merged 2 commits into from
Dec 21, 2021

Conversation

WatanabeToshimitsu
Copy link
Contributor

@WatanabeToshimitsu WatanabeToshimitsu commented Dec 20, 2021

What?

Fix build config for symlinking app common

Why?

app-common をシンボリックリンクしたときに,コンポーネントの見た目が違うものになっていて,ヴィジュアルリグレッションテストが正しく行えないから
原因は,app-common の peerDependencies に含まれるライブラリが重複し,MUI の theme が反映されなくなることにあると思われる

Screenshots or videos

app-common が npm でインストールされた場合

image

app-common/dist を node_modules/@dataware-tools/app-common にシンボリックリンクした場合

  • 修正前
    image

  • 修正後
    image

@hdl-service hdl-service requested a review from yusukefs December 20, 2021 15:23
@hdl-service hdl-service added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 20, 2021
@WatanabeToshimitsu WatanabeToshimitsu requested review from d-hayashi and removed request for yusukefs December 20, 2021 15:23
@hdl-service hdl-service added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Dec 20, 2021
@d-hayashi
Copy link
Contributor

なるほど。。
一応、app-common側でシンボリックリンクを貼らずに package.json の dependency の @dataware-tools/app-common をローカルフォルダに置き換えて npm install する CI を組んでみたのですが、それでも visual-regression test で同じエラーが生じたのでこの修正は必須っぽいですね...

https://github.com/dataware-tools/app-common/runs/4591160924?check_suite_focus=true

@d-hayashi
Copy link
Contributor

/lgtm

@hdl-service hdl-service added the lgtm Indicates that a PR is ready to be merged. label Dec 21, 2021
@hdl-service
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: d-hayashi, WatanabeToshimitsu

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [WatanabeToshimitsu,d-hayashi]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@hdl-service hdl-service merged commit 347c38d into master Dec 21, 2021
@hdl-service hdl-service deleted the fix/build-config-for-symlinking-app-common branch December 21, 2021 06:03
@WatanabeToshimitsu
Copy link
Contributor Author

@d-hayashi
マージしちゃった後に言うのアレなんですが,CI 通すだけならシンボリックリンクにこだわらずに,普通にファイルをコピーすれば通せることに気づきました 少なくともローカルでは上手く行ってます
これ以上D林さんの手を煩わせるのもアレなので,僕の方で CI 弄っちゃっても良いと思ってるんですが,どうしましょう...?

尚,この PR で追加された設定に関しては app-common の開発中に活きるはずですし,シンボリックリンクを用いない限り影響を受けはずないので,この設定自体を戻す必要はないと考えてます

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants